Modify

Opened 3 weeks ago

Last modified 5 days ago

#19875 new defect

Inactive Map Paint styles cause bad performance

Reported by: GerdP Owned by: team
Priority: normal Milestone: 20.10
Component: Core Version:
Keywords: template_report performance memory Cc:

Description (last modified by 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

Attachments (5)

1985-clear.patch (643 bytes) - added by GerdP 3 weeks ago.
free memory for rules if only loading meta data
19875.patch (3.7 KB) - added by GerdP 3 weeks ago.
POC: stop parsing when first non-meta rule was read
19875-remove-media.patch (3.8 KB) - added by GerdP 12 days ago.
19875.2.patch (3.4 KB) - added by GerdP 5 days ago.
updated for r17213
19875.3.patch (4.7 KB) - added by GerdP 5 days ago.
report timings when style is inactive

Download all attachments as: .zip

Change History (24)

comment:1 Changed 3 weeks ago by GerdP

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

comment:2 Changed 3 weeks ago by Klumbumbus

Keywords: performance added

comment:3 Changed 3 weeks ago by GerdP

Description: modified (diff)

comment:4 Changed 3 weeks ago by 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.

Changed 3 weeks ago by GerdP

Attachment: 1985-clear.patch added

free memory for rules if only loading meta data

comment:5 Changed 3 weeks ago by 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.

comment:6 in reply to:  5 Changed 3 weeks ago by 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.

Changed 3 weeks ago by GerdP

Attachment: 19875.patch added

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

comment:7 Changed 3 weeks ago by stoecker

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

comment:8 Changed 3 weeks ago by GerdP

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

comment:9 Changed 3 weeks ago by Don-vip

Keywords: memory added
Milestone: 20.10

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

comment:10 Changed 13 days ago by GerdP

In 17112/josm:

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

comment:11 in reply to:  9 Changed 13 days ago by 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.

comment:12 Changed 12 days ago by 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?

comment:13 Changed 12 days ago by Don-vip

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

comment:14 Changed 12 days ago by GerdP

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

comment:15 Changed 12 days ago by GerdP

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

Changed 12 days ago by GerdP

Attachment: 19875-remove-media.patch added

comment:16 Changed 12 days ago by 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?

comment:17 in reply to:  12 Changed 12 days ago by Klumbumbus

Replying to GerdP:

Do we still need this support?

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

comment:18 Changed 6 days ago by GerdP

In 17208/josm:

see #19875: Inactive Map Paint styles cause bad performance

  • remove support for deprecated @media in mapcss parser

Changed 5 days ago by GerdP

Attachment: 19875.2.patch added

updated for r17213

Changed 5 days ago by GerdP

Attachment: 19875.3.patch added

report timings when style is inactive

comment:19 Changed 5 days ago by 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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set.
to The owner will be changed from team to the specified user.
The owner will change to GerdP
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket
The owner will be changed from team to anonymous.

Add Comment


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

 
Note: See TracTickets for help on using tickets.