Modify

Opened 2 years ago

Closed 21 months ago

#22030 closed defect (wontfix)

[PATCH] Replace Utils.isStripEmpty with Utils.isBlank in AbstractPrimitive#put

Reported by: taylor.smock Owned by: team
Priority: normal Milestone: 22.07
Component: Core Version:
Keywords: performance Cc:

Description (last modified by taylor.smock)

I've been doing some profiling with Vector tiles, and I've noticed that isStripEmpty makes a significant number of allocations.

Looking at the profiler output, it looks like Utils.isBlank effectively gets optimized away (tested on Java 8/17). Does anyone know of a reason to keep Utils.isStripEmpty?

  • src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java

    diff --git a/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java b/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java
    index b4b1e1e339..92b1572a11 100644
    a b public abstract class AbstractPrimitive implements IPrimitive, IFilterablePrimit  
    584584    @Override
    585585    public void put(String key, String value) {
    586586        Map<String, String> originalKeys = getKeys();
    587         if (key == null || Utils.isStripEmpty(key))
     587        if (key == null || Utils.isBlank(key))
    588588            return;
    589589        else if (value == null) {
    590590            remove(key);

Utils#isStripEmpty (Java 8)
Utils#isStripEmpty, Java 8

Allocations:

  • Utils#isStripEmpty: 112 MB
  • Arrays#copyOf: 57 MB
  • AbstractPrimitive#getKeys: 25 MB
  • String[]: 4 MB

Utils#isBlank (Java 8)
Utils#isBlank, Java 8

Allocations:

  • Arrays#copyOf: 61 MB
  • AbstractPrimitive#getKeys: 3.5 MB
  • String[]: 2.3 MB

Utils#isBlank (Java 17)
Utils#isBlank, Java 17

Allocations:

  • Arrays#copyOf: 21 MB
  • AbstractPrimitive#getKeys: 43 MB
  • String[]: 12 MB

There is a bit of jitter, but the primary cost of AbstractPrimitive#put is from Utils#isStripEmpty.

Attachments (3)

flamegraph_isStripEmpty.png (76.3 KB ) - added by taylor.smock 2 years ago.
Utils#isStripEmpty (Java 8)
flamegraph_isBlank_Java17.png (86.1 KB ) - added by taylor.smock 2 years ago.
Utils#isBlank (Java 17)
flamegraph_isBlank_Java8.png (79.7 KB ) - added by taylor.smock 2 years ago.
Utils#isBlank (Java 8)

Download all attachments as: .zip

Change History (8)

by taylor.smock, 2 years ago

Attachment: flamegraph_isStripEmpty.png added

Utils#isStripEmpty (Java 8)

by taylor.smock, 2 years ago

Utils#isBlank (Java 17)

by taylor.smock, 2 years ago

Utils#isBlank (Java 8)

comment:1 by taylor.smock, 2 years ago

Description: modified (diff)

comment:2 by taylor.smock, 2 years ago

Description: modified (diff)

comment:3 by taylor.smock, 23 months ago

Milestone: 22.06
Summary: [PATCH][RFC] Replace Utils.isStripEmpty with Utils.isBlank in AbstractPrimitive#put[PATCH] Replace Utils.isStripEmpty with Utils.isBlank in AbstractPrimitive#put

comment:4 by stoecker, 22 months ago

Milestone: 22.0622.07

comment:5 by taylor.smock, 21 months ago

Resolution: wontfix
Status: newclosed

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. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.