[PATCH] MapCSS style cache should be dependent on ElemStyles instance

[zkir]

Use Case

The UbanEye3D plugin displays the active dataset in its own window with MapCSS style which can be different from the MapCSS style used in the main JOSM map view window.

(picture: see attachments).

The issue in JOSM with MapCSS styles.

In JOSM, MapCSS style (style set) is represented as an instance of ElemStyles class. There can be several such instances. However, currently there is only one cache for all instances of ElemStyles, which leads to the problem that only one style can be applied to all windows, and sometime crashes are produced, if different styles are specified.

Suggested solution

Make the style cache dependent on ElemStyles instance:

• StyleCache mappaintStyle in OsmPrimitive and VectorPrimitive becomes HashMap<ElemStyles,StyleCache> mappaintStyle.
• Parameters of methods of Stylable interface are adjusted accordingly. Stylable.getCachedStyle() becomes Stylable.getCachedStyle(ElemStyles styles), e.t.c.

Performance and memory impact

Performance impact: Performance tests via CLI DO NOT reveal any performance degradation (See detailed tests result in the comments below), which is expected, HashMap with just several items work quite fast.

Memory impact: Additional 48 bytes are consumed by the newly added HashMap per OsmPrimitive. For comparison, existing StyleCache field consumes 24 bytes (per OsmPrimitive).

References

• UrbanEye3D branch with the feature in question can be found here: ​https://github.com/Zkir/UrbanEye3D/tree/mapcss-layer

(Obviously it cannot be released until this issue in JOSM is fixed)

[stoecker]

Did you have a look at performance changes when not using UrbanEye?

[zkir]

Well, performance tests that I've managed to do -- via CLI -- do not reveal any performance degradation. See below.
I'd be very surprised if that were the case, because java HashMap with just several (1,2,3 or so) items in it should not affect performance anyway.

Zoom 16, 950x540

running rendering on the patched version
d:\src\josm>java -jar target\josm-1.5-SNAPSHOT.jar render -i d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm -s d:\UrbanEye3D\src\main\resources\mapcss-styles\urbaneye2d.mapcss -z 16 -o image.png
Rendering d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm to image.png
Rendering d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm to image.png completed in 1,5 s

running rendering on the original version
d:\src\josm>java -jar d:\tools\josm\josm-tested-orig.jar render -i d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm -s d:\UrbanEye3D\src\main\resources\mapcss-styles\urbaneye2d.mapcss -z 16 -o image-orig.png
Rendering d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm to image-orig.png
Rendering d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm to image-orig.png completed in 1,5 s

Zoom 19, 7680x4320

running rendering on the patched version
d:\src\josm>java -jar target\josm-1.5-SNAPSHOT.jar render -i d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm -s d:\UrbanEye3D\src\main\resources\mapcss-styles\urbaneye2d.mapcss -z 19 -o image.png
Rendering d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm to image.png
Rendering d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm to image.png completed in 3,1 s

running rendering on the original version
d:\src\josm>java -jar d:\tools\josm\josm-tested-orig.jar render -i d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm -s d:\UrbanEye3D\src\main\resources\mapcss-styles\urbaneye2d.mapcss -z 19 -o image-orig.png
Rendering d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm to image-orig.png
Rendering d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm to image-orig.png completed in 3,1 s

Zoom 20, 15360x8640

running rendering on the patched version
d:\src\josm>java -jar target\josm-1.5-SNAPSHOT.jar render -i d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm -s d:\UrbanEye3D\src\main\resources\mapcss-styles\urbaneye2d.mapcss -z 20 -o image.png
Rendering d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm to image.png
Rendering d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm to image.png completed in 6,8 s

running rendering on the original version
d:\src\josm>java -jar d:\tools\josm\josm-tested-orig.jar render -i d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm -s d:\UrbanEye3D\src\main\resources\mapcss-styles\urbaneye2d.mapcss -z 20 -o image-orig.png
Rendering d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm to image-orig.png
Rendering d:\UrbanEye3D\src\test\resources\osm_test_files\city_center.osm to image-orig.png completed in 6,8 s

No any idea how to do performance test imitating interactive user. If you have one, please advise. :)

[stoecker]

Well, any modification on the core elements is relevant. You also increase the memory footprint with this change, which is relevant. Can you check how much?

On first Test: You need to fix the tests, e.g.
test/functional/org/openstreetmap/josm/gui/mappaint/StyleCacheTest.java

[zkir]

Memory consumption measurements added to issue description.

[zkir]

patch including both code changes and autotests uplift

[zkir]

The patch file has been updated.
Changes: autotests, including functional one (StyleCacheTest.java) and command autotests (AddPrimitivesCommandTest.java etc) have been fixed.

[stoecker]

In 19528/josm:

see #24637 - patch by zkir (modified a bit) - allow to handle more than one style in caching

[stoecker]

I consider it important enough to have the size drawback and also the breaking plugins.

This will break MapRoulette and Mapillary plugins. Please provide updates for these as well. Also set new josm version to 19528 in build.xml :-)

May break others, but these aren't under JOSM control, so I don't know about them :-)

[zkir]

@stoecker, great news.

MapRoulette and Mapillary plugins. Please provide updates for these as well

Ok, I'll do that.