Opened 9 years ago

Last modified 9 years ago

#13309 closed enhancement

Caching and notifying preferences — at Version 14

Reported by: michael2402 Owned by: michael2402
Priority: normal Milestone: 17.04
Component: Core Version:
Keywords: gsoc-core Cc: Don-vip, bastiK, stoecker

Description (last modified by michael2402)

Currently, there are many MapFrame.repaint() calls whenever properties change.

Part of this is because colors and other values are cached by each component seperately.

I added a new infrastructure that allows you to get a property and easily cache that result.

To get a property use:

new IntegerProperty("my.property", "123").get()

To cache the result, use:

prop = new IntegerProperty("my.property", "123").cached(); // registers 123 as default value
prop.get(); // lazy get and convert to int
prop.get(); // get cached integer object

I added a new preference listener that listens to changes of one specific key to make this efficient.

Change History (19)

by michael2402, 9 years ago

comment:1 by simon04, 9 years ago

Description: modified (diff)

comment:2 by Don-vip, 9 years ago

Patch does not apply anymore, can you please update it?

by michael2402, 9 years ago

Attachment: patch-fix-13309.patch added

comment:3 by Don-vip, 9 years ago

    public static class ValueChangeEvent<T> {
        private final PreferenceChangeEvent base;

        private final AbstractProperty<T> source;

        ValueChangeEvent(PreferenceChangeEvent base, AbstractProperty<T> source) {
            this.base = base;
            this.source = source;
        }

base is not used?

comment:4 by Don-vip, 9 years ago

Why is the timeout commented in JOSMTestRules?

           //  statement = new FailOnTimeoutStatement(statement, timeout);

comment:5 by Don-vip, 9 years ago

In 10824/josm:

see #13309 - Caching and notifying preferences (patch by michael2402) - gsoc-core

comment:6 by michael2402, 9 years ago

Thanks for finding that timeout. No, it should not be commented. I did this for debugging.

comment:7 by Don-vip, 9 years ago

Resolution: fixed
Status: newclosed

ok this is fixed, I didn't apply this part.

comment:8 by Don-vip, 9 years ago

Resolution: fixed
Status: closedreopened
Summary: [Patch] Caching and notifying preferencesCaching and notifying preferences

this is not, this patch cause 74 unit test failures: jenkins/job/JOSM/1725/testReport/

comment:9 by Don-vip, 9 years ago

Owner: changed from team to michael2402
Status: reopenednew

comment:10 by Don-vip, 9 years ago

In 10839/josm:

see #13309 - remove deprecated color stuff not used outside of JOSM

comment:11 by Don-vip, 9 years ago

In 10853/josm:

see #13309 - fix most of deprecation warnings

comment:12 by Don-vip, 9 years ago

it seems the exception causing all these test failures is

java.lang.NullPointerException
	at org.openstreetmap.josm.data.preferences.AbstractToStringProperty.getAsString(AbstractToStringProperty.java:131)
	at org.openstreetmap.josm.data.preferences.AbstractToStringProperty.get(AbstractToStringProperty.java:92)
	at org.openstreetmap.josm.data.preferences.BooleanProperty.get(BooleanProperty.java:21)
	at org.openstreetmap.josm.tools.date.DateUtils.getDateTimeFormat(DateUtils.java:311)
	at org.openstreetmap.josm.gui.dialogs.NotesDialog$NoteRenderer.<init>(NotesDialog.java:209)
	at org.openstreetmap.josm.gui.dialogs.NotesDialog$NoteRenderer.<init>(NotesDialog.java:206)
	at org.openstreetmap.josm.gui.dialogs.NotesDialog.buildDialog(NotesDialog.java:85)
	at org.openstreetmap.josm.gui.dialogs.NotesDialog.<init>(NotesDialog.java:78)
	at org.openstreetmap.josm.gui.MapFrame.<init>(MapFrame.java:264)
	at org.openstreetmap.josm.gui.MainPanel.createNewMapFrame(MainPanel.java:88)
	at org.openstreetmap.josm.gui.MainPanel.updateContent(MainPanel.java:63)
	at org.openstreetmap.josm.gui.MainPanel$1.beforeFirstLayerAdded(MainPanel.java:157)
	at org.openstreetmap.josm.gui.layer.MainLayerManager.realAddLayer(MainLayerManager.java:264)
	at org.openstreetmap.josm.gui.layer.LayerManager.lambda$addLayer$0(LayerManager.java:180)
	at org.openstreetmap.josm.gui.util.GuiHelper.runInEDTAndWaitWithException(GuiHelper.java:138)
	at org.openstreetmap.josm.gui.layer.LayerManager.addLayer(LayerManager.java:180)
	at org.openstreetmap.josm.JOSMFixture.setupGUI(JOSMFixture.java:154)
	at org.openstreetmap.josm.JOSMFixture.access$000(JOSMFixture.java:28)
	at org.openstreetmap.josm.JOSMFixture$1.run(JOSMFixture.java:128)

comment:13 by michael2402, 9 years ago

The problem is that some class loads DateUtils before Main.pref is instantiated.

I suggest fixing this and adding a dependency on preferences for that test. I'll find the test that causes this and post a patch for it.

An alternative to make our life easier would be to make Main.pref a final field instantiated with an empty instance. We can then simply load it during program startup. This would also make early application startup much easier, since we currently have many classes that may only be loaded after Main.pref is set. We should make those classes listen to preferences updates though, but this is why I created this patch.

comment:14 by michael2402, 9 years ago

Description: modified (diff)

by michael2402, 9 years ago

Attachment: patch-latlon-generic.patch added

by michael2402, 9 years ago

Attachment: patch-fix-13309-2.patch added

by michael2402, 9 years ago

Attachment: patch-fix-13309-2.2.patch added
Note: See TracTickets for help on using tickets.