Inactive Map Paint styles cause bad performance

[GerdP]

What steps will reproduce the problem?

1. Have JOSM default style and a lot of (complex) inactive(!) Map Paint styles in preferences
2. Add a non-complex Map Paint style in the preferences dialog, e.g. the LIT style
3. Stop/Start JOSM

What is the expected result?

Quick reaction on 2. and no long delay in start phase

What happens instead?

Long delay and high CPU activity

Please provide any additional information below. Attach a screenshot if possible.

Watch the timestamps!

2020-10-02 10:04:40.365 FINE: Initializing map style resource://styles/standard/elemstyles.mapcss completed in 37 ms
2020-10-02 10:04:56.758 FINE: Initializing map style https://josm.openstreetmap.de/josmfile?page=Styles/Lit&zip=1 completed in 1 ms

I think problem is that the inactive styles are completely parsed (including lots of String.intern() actions although only the "meta" section is wanted. Might be a regression of #18749
Note that this was tested with a local build using r17083 although the reported version in the log below says r17080.

Build-Date:2020-10-02 09:36:29
Revision:17080
Is-Local-Build:true

Identification: JOSM/1.5 (17080 SVN en) Windows 10 64-Bit
OS Build number: Windows 10 Home 2004 (19041)
Memory Usage: 1987 MB / 3641 MB (1072 MB allocated, but free)
Java version: 1.8.0_191-b12, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920x1080 (scaling 1.0x1.0)
Maximum Screen Size: 1920x1080
Best cursor sizes: 16x16 -> 32x32, 32x32 -> 32x32
VM arguments: [-ea, -Dfile.encoding=UTF-8]
Program arguments: [--debug]

Plugins:
+ OpeningHoursEditor (35414)
+ PolygonCutOut (v0.7)
+ apache-commons (35524)
+ buildings_tools (35563)
+ continuosDownload (91)
+ ejml (35313)
+ geotools (35169)
+ gridify (1588746833)
+ jaxb (35092)
+ jts (35122)
+ merge-overlap (35248)
+ o5m (35248)
+ opendata (35513)
+ pbf (35446)
+ poly (35248)
+ reverter (35556)
+ terracer (35560)
+ undelete (35521)
+ utilsplugin2 (35487)

Map paint styles:
- https://josm.openstreetmap.de/josmfile?page=Styles/Ph_Typhoon&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/SlovakiaBicycleRoutes&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/ColourGPSData&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/ColourTag&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_buildings&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Kerbs&zip=1
- https://raw.githubusercontent.com/yopaseopor/traffic_signs_style_JOSM/master/Styles_Traffic_signs_AFR.zip
- https://raw.githubusercontent.com/yopaseopor/traffic_signs_style_JOSM/master/Styles_Traffic_signs_AME.zip
- https://raw.githubusercontent.com/yopaseopor/traffic_signs_style_JOSM/master/Styles_Traffic_signs_EUR_OC.zip
- https://raw.githubusercontent.com/yopaseopor/traffic_signs_style_JOSM/master/Styles_Traffic_signs_EUR_OR.zip
- https://raw.githubusercontent.com/yopaseopor/traffic_signs_style_JOSM/master/Styles_Traffic_signs_PAC.zip
+ https://josm.openstreetmap.de/josmfile?page=Styles/Lit&zip=1

[GerdP]

The inactive styles also eat up a lot of memory. A memory dump is ~400MB with them, only ~96 without.

[GerdP]

My understanding is that you want an inactive style because you want to be able to easily switch styles using the View -> Map Paint Styles action. The current implementation is quite strange.
It loads and stores the complete inactive style accept for some helping structrues. When the style is activated it doesn't use any of the stored information, it loads the complete style again.
So, I see these possible enhancements:
1) change mapcss.parsergen.MapCSSParser.sheet() so that it ignores data which is not needed when only metadata is wanted
This should reduce run time and memory footprint.
2) if 1) is too difficult, either make use of the already available data in field rules when a style is activated or clear the field when the style is not active.

[GerdP]

free memory for rules if only loading meta data

[GerdP]

I don't yet understand why meta data is extracted from the source. JOSM downloads the file josm.openstreetmap.de_styles which contains both the title and the icon for the style. Why not use that info?
Up to now I see only one possible exception: a local style which doesn't appear in the list.

[stoecker]

Replying to GerdP:

I don't yet understand why meta data is extracted from the source. JOSM downloads the file josm.openstreetmap.de_styles which contains both the title and the icon for the style. Why not use that info?
Up to now I see only one possible exception: a local style which doesn't appear in the list.

Exactly. The external styles list isn't the final instance. Local or also remote styles can be added separately.

[GerdP]

POC: stop parsing when first non-meta rule was read

[stoecker]

Indenting of patch looks strange (using forbidden tab stops?).

[GerdP]

You mean MapCSSParser.jj? My Eclipse doesn't help with the indention.

[Don-vip]

Sounds like a problem we clearly want to fix in next version :)

[GerdP]

In 17112/josm:

see #19875: Inactive Map Paint styles cause bad performance
free memory for rules if only loading meta data

[GerdP]

Replying to Don-vip:

Sounds like a problem we clearly want to fix in next version :)

two problems with 19875.patch:

• it relies on the presumption that the "meta" rules are at the beginning of the file
• it still calls preprocessor.pp_root() which also requires some seconds to process large map paint styles like Styles_Traffic_signs_AME.zip. I've not yet understood what this preprocessor does.

I think a proper solution would be to have a specialized parser that just collects the meta rules.

[GerdP]

@team: IIGTR the usage of @media in mapcss sources is deprecated since r8087, but we still execute the correspondend preprocessor when parsing mapcss sources. Since r9336 JOSM logs a warning.
Do we still need this support?

[Don-vip]

5 years of deprecation notice is long enough, I guess we can drop the support for good.

[GerdP]

OK, I'll prepare a patch which also removes the support from MapCSSParser.jj

[GerdP]

Arg, sorry, I just learned that the prepro also handles @supports, so we cannot simply remove it.

[GerdP]

I guess it is not possible to change the preprocessor so that it ignores all but the meta rules for the case that metadataOnly==true?

[Klumbumbus]

Replying to GerdP:

Do we still need this support?

I think I've never seen a style with @media.

[GerdP]

In 17208/josm:

see #19875: Inactive Map Paint styles cause bad performance

• remove support for deprecated @media in mapcss parser

[GerdP]

updated for r17213

[GerdP]

report timings when style is inactive

[GerdP]

The severe memory leak was fixed with r17112, but the run time penalty with inactive mappaint styles still exists, esp. when you change anything in the mappaint settings and press OK. All mapcss files are parsed again, also the inactive ones.

Some numbers (scroll right to see the timings):
With deactivated optimization:

2020-10-16 09:07:07.022 FINE: Initializing map style resource://styles/standard/elemstyles.mapcss completed in 49 ms
2020-10-16 09:07:07.024 FINE: Parsing map style https://josm.openstreetmap.de/josmfile?page=Styles/Lit&zip=1 completed in 0 ms
2020-10-16 09:07:08.742 FINE: Parsing map style https://raw.githubusercontent.com/yopaseopor/traffic_signs_style_JOSM/master/Styles_Traffic_signs_AME.zip completed in 1.7 s
2020-10-16 09:07:08.900 FINE: Parsing map style https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1 completed in 158 ms
2020-10-16 09:07:08.932 FINE: Parsing map style https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Streets&zip=1 completed in 30 ms
2020-10-16 09:07:08.936 FINE: Parsing map style https://josm.openstreetmap.de/josmfile?page=Styles/Power&zip=1 completed in 4 ms

With the patch:

2020-10-16 09:20:49.257 FINE: Parsing map style resource://styles/standard/elemstyles.mapcss completed in 50 ms
2020-10-16 09:20:49.258 FINE: Initializing map style resource://styles/standard/elemstyles.mapcss completed in 51 ms
2020-10-16 09:20:49.260 FINE: Parsing map style https://josm.openstreetmap.de/josmfile?page=Styles/Lit&zip=1 completed in 1 ms
2020-10-16 09:20:49.694 FINE: Parsing map style https://raw.githubusercontent.com/yopaseopor/traffic_signs_style_JOSM/master/Styles_Traffic_signs_AME.zip completed in 433 ms
2020-10-16 09:20:49.710 FINE: Parsing map style https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1 completed in 16 ms
2020-10-16 09:20:49.714 FINE: Parsing map style https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Streets&zip=1 completed in 4 ms
2020-10-16 09:20:49.716 FINE: Parsing map style https://josm.openstreetmap.de/josmfile?page=Styles/Power&zip=1 completed in 1 ms

Typically times for inactive styles are reduced to 30%. So, if only the indention is a problem, please improve the patch.

[Don-vip]

Milestone renamed

[stoecker]

Milestone renamed