[Patch] Initialize Territories+RightAndLefthandTraffic together, remove "Edit boundaries", save memory

[simon04]

Can we remove "Edit boundaries" from the advanced preferences? If yes, we could get rid of the DataSet from boundaries.osm on the heap after initialization and spare ≈700kB of memory.

org.openstreetmap.josm.data.osm.DataSet#1		230 B (0%)	658,890

[Don-vip]

Replying to simon04:

Can we remove "Edit boundaries" from the advanced preferences?

Do you see a way to maintain the file without it? I didn't.

[Don-vip]

You can make it an advanced preference disabled by default, but I need to be able to edit the file.

[simon04]

I just opened boundaries.osm from the checked out JOSM source code. I wasn't aware of this feature until today.

[Don-vip]

Replying to simon04:

I just opened boundaries.osm from the checked out JOSM source code.

This way all ids will change when you save the file back, so it doesn't work. Another option would be to add a command-line argument disabling the initialization of territories, so we could open the file from the filesystem, at the loss of some functionalities.

[simon04]

In 16321/josm:

fix #18907 - Initialize Territories+RightAndLefthandTraffic together, remove "Edit boundaries" to save memory (unless started with --debug)

[simon04]

Fixed pt_assistant via ​https://github.com/JOSM/pt_assistant/commit/7d9bba30ec1e9e7d6d5c9c1dc61dee93b05c89e0

[taylor.smock]

@simon04: Is there any chance we can make modifications to the class so that we can get information that is removed (using the KNOWN_KEYS list)? I.e., something like Territories.getAllTags(String code). I know you made this change to save memory, and I don't know if keeping the tags in the KNOWN_KEYS list will make the change counter productive.

I had been using it to map names that MapWithAI was giving me to locations (so Alaska -> US-AL). Since Territories now sets the DataSet to null, if I call Territories.getOriginalDataSet, I get a NPE (I had been using Territories.getDataSet, but that was removed in the same changeset that defaults to setting the Territories dataset to null).

I had (essentially) been doing Territories.getOriginalDataSet().allPrimitives().parallelStream().filter(o -> o.hasKey("name:en") && o.get("name:en").equalsIgnoreCase(countryName)).findAny() to take a country name and map it to a location.

If all else fails, I can (manually) make modifications to an (out of date) "copy" I made in the MapWithAI repo. It will probably be out of date every so often though. :(

Note: The plugin reads ​https://github.com/facebookmicrosites/Open-Mapping-At-Facebook/blob/master/data/rapid_releases.geojson for data.

[simon04]

Hi taylor.smock, sorry for breaking your plugin. I think we can spare the memory for 435 names of countries/states (previously we've been keeping the metadata of hundreds of nodes). Would you provide a patch for the relevant changes best fitting your needs?

[taylor.smock]

Replying to simon04:

Hi taylor.smock, sorry for breaking your plugin. I think we can spare the memory for 435 names of countries/states (previously we've been keeping the metadata of hundreds of nodes). Would you provide a patch for the relevant changes best fitting your needs?

Don't worry (too much) about breaking my plugin -- you couldn't have known I had been using the Territories Dataset. I know that JOSM core can really only care about the plugins that the JOSM core devs can fix/make modifications to. Its one of the reasons I want to move my plugins into the JOSM organization sometime, but I'm waiting on the move to gitlab with #16857.

I can go ahead and make a patch if it is still desired. That being said, I no longer need it -- I didn't get a reply fast enough, and the next milestone was due soon, so I took the time to update my (naive) copy of the data and rip out the code that took the originating rapid_releases.geojson and converted it into a format usable for the plugin.

Since I no longer need the functionality, do you think it would be worth it to keep the data anyway? (ISO3166_1, ISO3166_2, TAGINFO, "type", "name:en", "driving_side", "note") Probably the big ones to keep would be the ISO3166 and name:en tags.

[simon04]

Most of the time I check for usages in plugins before changing an API, sometimes I forget, though.

The ISO3166 codes go into the keys of Map<String, TagMap> customTagsCache, so I don't know, whether we should keep them? The name:en seem to be of interest.

Thus, I'll drop "name:en" from KNOWN_KEYS?

[taylor.smock]

That is probably the easiest and simplest solution. It also doesn't have as much overhead as creating an entirely new variable to hold them.

Something to keep in mind is that the customTagsCache currently has very few entries relative to the number of countries in the world. The only profiler I have access to is VisualVM, and that doesn't really tell me what is using all the memory (it might say int[] uses 133MB, but it doesn't break down what classes use it), so I have no idea how that will affect memory usage.

Again, as far as checking for usages in plugins before changing an API, the MapWithAI plugin isn't cloned as part of the JOSM source tree (when you get the plugins as well), so I would not expect you (or anyone else) to know that you broke the plugin.

[simon04]

Retained size of customTagsCache increases from 4_120 to 38_112 when including "name:en". Still a viable amount, I'd say.

[taylor.smock]

So the net memory gain should be 658_890 - (38_112 - 4_120), or 624_898 if name:en is kept. I guess 625 KB is a win. :)

[simon04]

In 16381/josm:

see #18907 - Territories: retain "name:en" for custom tags

[Klumbumbus]

Milestone renamed

[simon04]

pt_assistant updated via PluginsSource?action=diff&version=538