Modify

Opened 3 months ago

Closed 2 months ago

Last modified 2 months ago

#18871 closed defect (fixed)

Performance issues after updating

Reported by: ukash63@… Owned by: team
Priority: major Milestone: 20.03
Component: Core Version:
Keywords: template_report regression performance Cc: simon04

Description

Description

After upgrading JOSM from version 15806 to 15937 there is noticeable performance loss when moving nodes, ways or sometimes when adding tags. I noticed higher cpu usage. Downgrading to previous version (15806) fixes problem.
I have tested it on two different computers (with the same operating system).

Please provide any additional information below. Attach a screenshot if possible.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-02-26 10:50:27 +0100 (Wed, 26 Feb 2020)
Revision:15937
Build-Date:2020-02-26 09:52:41
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (15937 pl) Linux Arch Linux
Memory Usage: 207 MB / 4008 MB (92 MB allocated, but free)
Java version: 13.0.2+8, N/A, OpenJDK 64-Bit Server VM
Screen: :0.0 2560x1440
Maximum Screen Size: 2560x1440

Attachments (3)

josm-comp.mp4 (11.0 MB) - added by ukash63@… 3 months ago.
perfo.jpg (235.6 KB) - added by Don-vip 2 months ago.
selection.jpg (13.8 KB) - added by Don-vip 2 months ago.

Change History (27)

Changed 3 months ago by ukash63@…

Attachment: josm-comp.mp4 added

comment:1 Changed 3 months ago by Klumbumbus

Keywords: regression performance added
Priority: normalmajor

comment:2 Changed 3 months ago by Don-vip

Can you please test various snapshots between r15806 and r15937 to help us to narrow the range of revisions where the problem was introduced? You can download them here: https://josm.openstreetmap.de/download/

comment:3 Changed 3 months ago by Don-vip

Milestone: 20.03

comment:4 in reply to:  3 Changed 3 months ago by ukash63@…

I have tested and problem was introduced in snapshot 15889.

Snapshop 15887 does not have this issue.

comment:5 Changed 3 months ago by GerdP

Cc: simon04 added

Changes in r15889 were done for #14088

comment:6 Changed 3 months ago by simon04

I've performed a performance test: for every primitive in source:trunk/nodist/data/neubrandenburg.osm.bz2, an image is obtained. Here are the results for r15806, r15888, r16053:

  • test/performance/org/openstreetmap/josm/gui/mappaint/MapRendererPerformanceTest.java

     > git diff
    diff --git a/test/performance/org/openstreetmap/josm/gui/mappaint/MapRendererPerformanceTest.java b/test/performance/org/openstreetmap/josm/gui/mappaint/MapRendererPerformanceTest.java
    index c20fe1ca1..6b2479f40 100644
    a b public static void cleanUp() { 
    170174        MapPaintStyleLoader.reloadStyles(defaultStyleIdx);
    171175    }
    172176
     177    @Test
     178    public void testImageProviderGetPadded() {
     179        final LongSummaryStatistics statistics = new LongSummaryStatistics();
     180        for (int i = 0; i < 3; i++) {
     181            final Stopwatch stopwatch = Stopwatch.createStarted();
     182            for (OsmPrimitive primitive : dsCity.allPrimitives()) {
     183                ImageProvider.getPadded(primitive, ImageProvider.ImageSizes.SMALLICON.getImageDimension());
     184                // if (primitive.isTagged()) MapPaintStyles.getNodeIcon(primitive.getKeys().getTags().iterator().next());
     185            }
     186            statistics.accept(stopwatch.elapsed());
     187        }
     188        System.out.println(statistics);
     189        // r15806 MapPaintStyles.getNodeIcon LongSummaryStatistics{count=3, sum=23827, min=4564, average=7942.333333, max=11388}
     190        // r15806 ImageProvider.getPadded    LongSummaryStatistics{count=3, sum=55536, min=18056, average=18512.000000, max=19324}
     191        // r15888 MapPaintStyles.getNodeIcon LongSummaryStatistics{count=3, sum=24332, min=5063, average=8110.666667, max=11515}
     192        // r15888 ImageProvider.getPadded    LongSummaryStatistics{count=3, sum=57233, min=18730, average=19077.666667, max=19731}
     193        // r16053 MapPaintStyles.getNodeIcon LongSummaryStatistics{count=3, sum=29961, min=6784, average=9987.000000, max=13779}
     194        // r16053 ImageProvider.getPadded    LongSummaryStatistics{count=3, sum=5354, min=1541, average=1784.666667, max=2245}
     195        // r16053 ImageProvider.getPadded    LongSummaryStatistics{count=20, sum=31170, min=1494, average=1558.500000, max=2200}
     196    }
     197
    173198    private static class PerformanceTester {
    174199        public double scale = 0;
    175200        public LatLon center = LL_CITY;

@team, any ideas/hints what might have caused this performance regression?

comment:7 Changed 3 months ago by stoecker

I'd say the only image related changes where your changes to SVG.

comment:8 Changed 3 months ago by GerdP

Sorry, can't help here. For me, both r15887 and r15889 are equally slow. No idea if this is relevant here: The objects which are moved are not the same. Different oneway roads are effected and maybe the arrows for them are important?

comment:9 Changed 3 months ago by ukash63@…

I've checked it on another computer (MacOS) and there's also this slowdown noticable. It happens even when I grab point that is not connected to anything.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-02-21 21:19:34 +0100 (Fri, 21 Feb 2020)
Revision:15889
Build-Date:2020-02-22 02:30:59
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (15889 pl) Mac OS X 10.15.2
OS Build number: Mac OS X 10.15.2 (19C57)
Memory Usage: 385 MB / 2048 MB (109 MB allocated, but free)
Java version: 13.0.2+8, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: Display 69732928 1680x1050, Display 1127222057 1920x1080, Display 1127222050 1920x1080
Maximum Screen Size: 1920x1080
Dataset consistency test: No problems found

comment:10 Changed 3 months ago by simon04

Err, since r15889, org.openstreetmap.josm.tools.ImageProvider#paddedImageCache uses Image as key, likely causing expensive hashCode computations. I'm trying to move this cache into the individual ImageResource instances.

comment:11 Changed 3 months ago by simon04

In 16060/josm:

see #18871 - ImageProvider.getPadded: fix/improve performance

comment:12 Changed 3 months ago by simon04

In 16062/josm:

see #18871 - fix MapCSSRendererTest.testRender[way-repeat-image]

comment:13 Changed 3 months ago by simon04

In 16074/josm:

fix #18884, see #18871 - ImageResource.imgCache: use ConcurrentHashMap

comment:14 Changed 3 months ago by Don-vip

@simon: Is the performance regression fixed?

comment:15 Changed 3 months ago by simon04

Resolution: fixed
Status: newclosed

Most likely since the issue described in comment:10 has been addressed.

comment:16 Changed 3 months ago by Don-vip

Resolution: fixed
Status: closedreopened

I just tried to move a single node, with a small data set, and it is extremely slow.

comment:17 Changed 3 months ago by Don-vip

r15887 is fine. r15889 is slow. r16173 is as slow as r15889, so the original issue is not fixed.

Changed 2 months ago by Don-vip

Attachment: perfo.jpg added

comment:18 Changed 2 months ago by Don-vip

When I move a tagged node, this is where most of the CPU is spent:


It likely comes from the update of selection list:


Last edited 2 months ago by Don-vip (previous) (diff)

Changed 2 months ago by Don-vip

Attachment: selection.jpg added

comment:19 Changed 2 months ago by GerdP

Would match my results. I have preference properties.presets.visible=false

comment:20 Changed 2 months ago by GerdP

No, sorry, forget that. The setting is not about the icon for the selected element.

comment:21 Changed 2 months ago by Don-vip

Resolution: fixed
Status: reopenedclosed

In 16186/josm:

fix #18871 - fix performance regression - padded icons requests made unnecessary costly rescale operations

comment:22 Changed 2 months ago by simon04

Vincent, big thanks for looking into and fixing this issue!

comment:23 Changed 2 months ago by Don-vip

You're welcome :) One further improvement would be to not refresh the padded icon in selection list for movement events. I'm not sure if it's doable easily.

comment:24 Changed 2 months ago by Don-vip

Ticket #19031 has been marked as a duplicate of this ticket.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
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.