Fix issues reported by Heap Hero

[simon04]

​https://heaphero.io/my-heap-report.jsp?p=YXJjaGl2ZWQvMjAyMC8wMi8yMi8tLWhlYXBkdW1wLTE1ODIzODQ0NjA5NjIuaHByb2YuZ3otMTUtMTYtNy5qc29uLS0=

The heap bump has been obtained from running JOSM r15898 on a fresh profile and loading the attached dataset.

Build-Date:2020-02-22 17:26:08
Revision:15898
Is-Local-Build:true

Identification: JOSM/1.5 (15898 SVN en_GB) Linux Arch Linux
Memory Usage: 829 MB / 3531 MB (658 MB allocated, but free)
Java version: 1.8.0_242-b08, Oracle Corporation, OpenJDK 64-Bit Server VM
Screen: :0.0 3840x2160
Maximum Screen Size: 3840x2160
VM arguments: [-Djosm.home=<josm.pref>/]
Program arguments: [--set=expert=true, --set=iso.dates=true]
Dataset consistency test: No problems found

[Don-vip]

Looks like svgSalamander is responsible for many of the issues.

[Don-vip]

What version of Java were you running?

[GerdP]

See kitfox.patch in #17040

[GerdP]

The svg issue is still open: ​https://github.com/blackears/svgSalamander/issues/36

[Don-vip]

Replying to GerdP:

The svg issue is still open: ​https://github.com/blackears/svgSalamander/issues/36

You should open a Pull Request instead.

[simon04]

With attachment:18749.patch​​ applied, the "5. Duplicate Strings / Wasted Memory" goes down from 9.7mb to ​4.64mb

[GerdP]

Replying to Don-vip:

Replying to GerdP:

The svg issue is still open: ​https://github.com/blackears/svgSalamander/issues/36

You should open a Pull Request instead.

I still don't know how to open a Pull Requests. I hope I don't have to learn it for JOSM, whenever I tried to work with git I was searching for the right way to undo something in my local directory and ended in messed up code. Getting old...
I think the negative comment regarding String.intern() usage stopped my issue.

[simon04]

Replying to GerdP:

I still don't know how to open a Pull Requests.

You might want to use the GitHub website to make an edit and create a PR: ​https://help.github.com/en/github/managing-files-in-a-repository/editing-files-in-another-users-repository

How's your svgSalamander workflow? We make modifications to our copy and at the same time try to get the changes upstream via a PR?

[Don-vip]

Replying to simon04:

We make modifications to our copy and at the same time try to get the changes upstream via a PR?

Yes. It can take many months before svgSalamander PR are merged, so we clearly don't want to wait.

[simon04]

In 15901/josm:

see #18749 - Intern strings to reduce memory footprint of svgSalamander

[simon04]

In 15902/josm:

see #18749 - Intern strings to reduce memory footprint

[simon04]

In 15904/josm:

see #18749 - SVGElement: store String instead of StyleAttribute in map

[simon04]

In 15909/josm:

see #18749 - Use efficient/unmodifiable collections

[GerdP]

Replying to simon04:

Replying to GerdP:

I still don't know how to open a Pull Requests.

You might want to use the GitHub website to make an edit and create a PR: ​https://help.github.com/en/github/managing-files-in-a-repository/editing-files-in-another-users-repository

Thanks for the link. That looks easy enough for me.

[simon04]

I submitted two upstream PR for svgSalamander (extracted from r15901 and r15904, respectively):

• Intern strings to reduce memory footprint by simon04 · Pull Request #52 · ​https://github.com/blackears/svgSalamander/pull/52
• SVGElement: store String instead of StyleAttribute in map by simon04 · Pull Request #53 · ​https://github.com/blackears/svgSalamander/pull/53

[simon04]

In 15912/josm:

see #18749 - Use efficient/unmodifiable maps for svgSalamander

[Don-vip]

Nice! Are there still issues to fix?

[simon04]

There are, for sure ;). But it shouldn't hinder us from releasing a new tested. I can always open a follow-up ticket for the next milestone.

[tuxayo]

Which kinds of memory savings have been seen during the testing of this patch? (just for the record)

[simon04]

The reports:

• r15898: ​https://heaphero.io/my-heap-report.jsp?p=YXJjaGl2ZWQvMjAyMC8wMi8yMi8tLWhlYXBkdW1wLTE1ODIzODQ0NjA5NjIuaHByb2YuZ3otMTUtMTYtNy5qc29uLS0= (Java version: 1.8.0_242-b08, Oracle Corporation, OpenJDK 64-Bit Server VM)
• r15925: ​https://heaphero.io/my-heap-report.jsp?p=YXJjaGl2ZWQvMjAyMC8wMi8yNC8tLWhlYXBkdW1wLTE1ODI1Nzc2MTYwMjkuaHByb2YuZ3otMjAtNTQtMzAuanNvbi0t (Java version: 1.8.0_242-b08, Oracle Corporation, OpenJDK 64-Bit Server VM)
• r15925: ​https://heaphero.io/my-heap-report.jsp?p=YXJjaGl2ZWQvMjAyMC8wMi8yNC8tLWhlYXBkdW1wLTE1ODI1Nzc0ODA0NDcuaHByb2YuZ3otMjAtNTItMTYuanNvbi0t (Java version: 15-ea+11-373, Oracle Corporation, OpenJDK 64-Bit Server VM)

The numbers:

• Object Headers 19.92 → 18.19mb
• Duplicate Strings 9.7 → 4.5mb
• Inefficient collections 6.4 → 5.4mb
• Duplicate Objects 2.3 → 1.2mb

[GerdP]

Last time that I looked at this I wondered why we keep both the parsed svg data and the rendered image in the heap, possibly also the svg source files in a cache.

[simon04]

In 15934/josm:

see #18749 - Intern strings to reduce memory footprint

[simon04]

In 15935/josm:

see #18749, see #16183 - Add non-regression test

[simon04]

In 15949/josm:

see #18749, see #16183 - removeAreaStylePseudoClass not for validator MapCSS files

[GerdP]

In 16031/josm:

see #18749: reduce memory footprint
errors array was allocated with 30 elements for each MapCSSTagCheckerAndRule instance

[GerdP]

FYI: I've noticed that the *.svg file in the svn repo are much larger than the ones included in the jar files produced by Jenkins. Seems that there is a compressor which removes lots of strings. No idea if that has an influence on the memory footprint?

[Klumbumbus]

Replying to GerdP:

Seems that there is a compressor which removes lots of strings.

Yes we use ​https://github.com/RazrFalcon/svgcleaner but a patched version iirc.

[GerdP]

I think r15909 introduced a performance regresssion. Field conds used to be null for empty collections, now it is never null. In Selector.MatchingReferrerFinder.doVisit() this means that a costly sequential search is performed much more often.
This "kills" JOSM when ways with many nodes are processed.

[GerdP]

In 16191/josm:

see #18749: fix performance regression introduced with r15909