Modify

Opened 10 years ago

Closed 8 years ago

#4161 closed defect (fixed)

[patch] Major slowdown in recent versions

Reported by: stoecker Owned by: Don-vip
Priority: major Milestone:
Component: Core Version:
Keywords: Cc:

Description

With a medium sized dadaset adding tags or e.g. adding virtual nodes takes very long. Some of the changes in last versions still have problems with larger datasets and these issues are now exposed to the user through normal all-day-operations.

Attachments (1)

new_patch.diff (3.0 KB) - added by Don-vip 8 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 10 years ago by jttt

This is caused by ChangesetsInActiveDataLayerListModel that updates list of changeset after every change in dataset (even when changset dialog is not shown). Best would be if the model used DatasetEventManager to register for current dataset events and update only what is necessary. Also ToggleDialog.showNotify and hideNotify methods should be used to register to listeners only when dialog is visible.

Problem is that change of changesetId doesn't generate dataset event yet.

I'll fix it on weekend if nobody gets time for that sooner.

comment:2 Changed 10 years ago by jttt

Owner: changed from team to jttt

comment:3 Changed 10 years ago by jttt

Resolution: fixed
Status: newclosed

(In [2655]) Fixed #4161 Major slowdown in recent versions, used correct pattern for listeners realized using CopyOnWriteArrayList

comment:4 in reply to:  3 Changed 8 years ago by Don-vip

Resolution: fixed
Status: closedreopened

Replying to jttt:

(In [2655]) Fixed #4161 Major slowdown in recent versions, used correct pattern for listeners realized using CopyOnWriteArrayList

I think this fix has still to be improved.
There is a problem in this piece of code from trunk/src/org/openstreetmap/josm/gui/dialogs/changeset/ChangesetListModel.java:

    protected void addChangeset(Changeset changeset) {
        if (shownChangesets.add(changeset)) {
            setChangesets(shownChangesets);
            updateModel();
        }
    }

    protected void removeChangeset(Changeset changeset) {
        if (shownChangesets.remove(changeset)) {
            setChangesets(shownChangesets);
            updateModel();
        }
    }

    protected void setChangesets(Collection<Changeset> changesets) {
        shownChangesets.clear();
        if (changesets != null) {
            shownChangesets.addAll(changesets);
        }
        updateModel();
    }

    private void updateModel() {
        Set<Changeset> sel = getSelectedChangesets();
        data.clear();
        data.addAll(shownChangesets);
        ChangesetCache cache = ChangesetCache.getInstance();
        for (Changeset cs: data) {
            ...
        }
        sort();
        fireIntervalAdded(this, 0, getSize());
        setSelectedChangesets(sel);
    }

Each call of addChangeset() and removeChangeset() has for sole effect to clear the collections shownChangesets and data ! In fact, these collections are only managed via the setChangeset() method.
These (useless?) methods are however called very often (at least at every primitive creation in edit layer) and may increase memory footprint for nothing.

Are you ok to delete them ? (see attached patch)

Last edited 8 years ago by Don-vip (previous) (diff)

Changed 8 years ago by Don-vip

Attachment: new_patch.diff added

comment:5 Changed 8 years ago by stoecker

Owner: changed from jttt to Don-vip
Status: reopenednew
Summary: Major slowdown in recent versions[patch] Major slowdown in recent versions

Do you handle this yourself?

comment:6 Changed 8 years ago by Don-vip

Yep, on my way :)

comment:7 Changed 8 years ago by Don-vip

Resolution: fixed
Status: newclosed

In [4572/josm]:

fix #4161 - Removed useless code

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Don-vip.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.