Modify

Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#18749 closed enhancement (fixed)

Fix issues reported by Heap Hero

Reported by: simon04 Owned by: simon04
Priority: normal Milestone: 20.02
Component: Core Version:
Keywords: memory heap intern heaphero svg svgsalamander Cc:

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

Attachments (2)

b.osm.bz2 (72.4 KB) - added by simon04 3 months ago.
18749.patch (9.6 KB) - added by simon04 3 months ago.

Download all attachments as: .zip

Change History (36)

Changed 3 months ago by simon04

Attachment: b.osm.bz2 added

comment:1 Changed 3 months ago by Don-vip

Keywords: svg added

Looks like svgSalamander is responsible for many of the issues.

comment:2 Changed 3 months ago by Don-vip

What version of Java were you running?

comment:3 Changed 3 months ago by GerdP

Keywords: svg removed

See kitfox.patch in #17040

comment:4 Changed 3 months ago by GerdP

Keywords: svg added

comment:5 Changed 3 months ago by GerdP

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

comment:7 Changed 3 months ago by simon04

Description: modified (diff)

Changed 3 months ago by simon04

Attachment: 18749.patch added

comment:8 Changed 3 months ago by simon04

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

comment:9 Changed 3 months ago by Don-vip

Milestone: 20.02

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

comment:11 in reply to:  9 ; Changed 3 months ago by 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?

comment:12 in reply to:  11 Changed 3 months ago by 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.

comment:13 Changed 3 months ago by simon04

In 15901/josm:

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

comment:14 Changed 3 months ago by simon04

In 15902/josm:

see #18749 - Intern strings to reduce memory footprint

comment:15 Changed 3 months ago by simon04

In 15904/josm:

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

comment:16 Changed 3 months ago by simon04

In 15909/josm:

see #18749 - Use efficient/unmodifiable collections

comment:17 in reply to:  11 Changed 3 months ago by 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.

comment:18 Changed 3 months ago by simon04

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

comment:19 Changed 3 months ago by simon04

In 15912/josm:

see #18749 - Use efficient/unmodifiable maps for svgSalamander


Here is a histogram of the sizes of the map fields in SVGElement when running with Java ⩾9:

  count      map in SVGElement   map.size   class used since r15912
  ===================================================================================
   4894      [java] inlineStyles     0000   class java.util.Collections$EmptyMap
   1297      [java] inlineStyles     0001   class java.util.Collections$SingletonMap
   1118      [java] inlineStyles     0002   class java.util.ImmutableCollections$MapN
    537      [java] inlineStyles     0003   class java.util.ImmutableCollections$MapN
    552      [java] inlineStyles     0004   class java.util.ImmutableCollections$MapN
    319      [java] inlineStyles     0005   class java.util.ImmutableCollections$MapN
    443      [java] inlineStyles     0006   class java.util.ImmutableCollections$MapN
    423      [java] inlineStyles     0007   class java.util.ImmutableCollections$MapN
    438      [java] inlineStyles     0008   class java.util.ImmutableCollections$MapN
    513      [java] inlineStyles     0009   class java.util.ImmutableCollections$MapN
    561      [java] inlineStyles     0010   class java.util.ImmutableCollections$MapN
    222      [java] inlineStyles     0011   class java.util.ImmutableCollections$MapN
     31      [java] inlineStyles     0012   class java.util.ImmutableCollections$MapN
      3      [java] inlineStyles     0013   class java.util.ImmutableCollections$MapN
      8      [java] inlineStyles     0014   class java.util.ImmutableCollections$MapN
      6      [java] inlineStyles     0015   class java.util.ImmutableCollections$MapN
     18      [java] inlineStyles     0016   class java.util.ImmutableCollections$MapN
     39      [java] inlineStyles     0017   class java.util.ImmutableCollections$MapN
     24      [java] inlineStyles     0018   class java.util.ImmutableCollections$MapN
     53      [java] inlineStyles     0019   class java.util.ImmutableCollections$MapN
     21      [java] inlineStyles     0020   class java.util.ImmutableCollections$MapN
     40      [java] inlineStyles     0021   class java.util.ImmutableCollections$MapN
      4      [java] inlineStyles     0022   class java.util.ImmutableCollections$MapN
      1      [java] inlineStyles     0027   class java.util.ImmutableCollections$MapN
    395      [java] inlineStyles     0028   class java.util.ImmutableCollections$MapN
     39      [java] inlineStyles     0029   class java.util.ImmutableCollections$MapN
      2      [java] inlineStyles     0031   class java.util.ImmutableCollections$MapN
      1      [java] inlineStyles     0032   class java.util.ImmutableCollections$MapN
      1      [java] inlineStyles     0033   class java.util.ImmutableCollections$MapN
      4      [java] inlineStyles     0036   class java.util.ImmutableCollections$MapN
      1      [java] inlineStyles     0049   class java.util.ImmutableCollections$MapN
     28      [java] inlineStyles     0050   class java.util.ImmutableCollections$MapN
     24      [java] presAttributes   0000   class java.util.Collections$EmptyMap
   1730      [java] presAttributes   0001   class java.util.Collections$SingletonMap
    596      [java] presAttributes   0002   class java.util.ImmutableCollections$MapN
   1568      [java] presAttributes   0003   class java.util.ImmutableCollections$MapN
   2161      [java] presAttributes   0004   class java.util.ImmutableCollections$MapN
   1624      [java] presAttributes   0005   class java.util.ImmutableCollections$MapN
   1516      [java] presAttributes   0006   class java.util.ImmutableCollections$MapN
   1428      [java] presAttributes   0007   class java.util.ImmutableCollections$MapN
    489      [java] presAttributes   0008   class java.util.ImmutableCollections$MapN
    537      [java] presAttributes   0009   class java.util.ImmutableCollections$MapN
    160      [java] presAttributes   0010   class java.util.ImmutableCollections$MapN
    134      [java] presAttributes   0011   class java.util.ImmutableCollections$MapN
     50      [java] presAttributes   0012   class java.util.ImmutableCollections$MapN
      4      [java] presAttributes   0013   class java.util.ImmutableCollections$MapN
      5      [java] presAttributes   0014   class java.util.ImmutableCollections$MapN
      9      [java] presAttributes   0015   class java.util.ImmutableCollections$MapN
      1      [java] presAttributes   0016   class java.util.ImmutableCollections$MapN
Last edited 3 months ago by simon04 (previous) (diff)

comment:20 Changed 3 months ago by Don-vip

Nice! Are there still issues to fix?

comment:21 Changed 3 months ago by 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.

comment:22 Changed 3 months ago by simon04

Resolution: fixed
Status: assignedclosed

comment:23 Changed 3 months ago by tuxayo

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

comment:24 Changed 3 months ago by simon04

The reports:

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

comment:25 Changed 3 months ago by 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.

comment:26 Changed 3 months ago by simon04

In 15934/josm:

see #18749 - Intern strings to reduce memory footprint

comment:27 Changed 3 months ago by simon04

In 15935/josm:

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

comment:28 Changed 3 months ago by simon04

In 15949/josm:

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

comment:29 Changed 3 months ago by GerdP

In 16031/josm:

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

comment:30 Changed 3 months ago by simon04

Keywords: svgsalamander added

comment:31 Changed 3 months ago by 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?

comment:32 in reply to:  31 Changed 3 months ago by 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.

comment:33 Changed 3 months ago by 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.

comment:34 Changed 3 months ago by GerdP

In 16191/josm:

see #18749: fix performance regression introduced with r15909

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain simon04.
as The resolution will be set.
The resolution will be deleted.

Add Comment


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

 
Note: See TracTickets for help on using tickets.