Changeset 18431 in josm for trunk/src


Ignore:
Timestamp:
2022-04-19T19:12:51+02:00 (3 years ago)
Author:
taylor.smock
Message:

VectorTile/MapBox/Feature: Significantly reduce allocations

The area tested can be downloaded with
--download=39.0637818,-108.5670233,39.0660809,-108.5620022.
VectorTile downloads was tested via the Mapillary plugin.

Differences (bytes, from IntelliJ Profiler in IDEA 2022.1):

Class Method Original New Difference
CommandInteger init 81,156,792 5,045,360 -76,111,432 (-93.7%)
Feature parseTagValue 354,995,472 143,200,680 -211,794,792 (-59.6%)
ProtobufPacked init 458,471,192 189,104,744 -269,366,448 (-58.7%)
ProtobufRecord init 492,731,416 142,510,112 -350,221,304 (-71.0%)
Feature init 1,437,014,520 550,452,376 -886,562,144 (-61.7%)

Most of the allocations came from the following sources:

  • Array copies
    • List#add (mostly fixed through the use of a ByteArrayOutputStream, as the lists were List<Byte>)
    • Enum#values (this is where the changes to Command and WireType come from)
  • Streams (fixed by using a traditional for loop)
  • NumberFormat#getNumberInstance (fixed by creating a static instance)
Location:
trunk/src/org/openstreetmap/josm/data
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/org/openstreetmap/josm/data/imagery/vectortile/mapbox/Command.java

    r17867 r18431  
    2323    ClosePath((byte) 7, (byte) 0);
    2424
     25    private static final Command[] CACHED_VALUES = Command.values();
    2526    private final byte id;
    2627    private final byte parameters;
    … …  
    4647        return this.parameters;
    4748    }
     49
     50    /**
     51     * Get a pre-calculated array of all {@link Command} values.
     52     * @return An array of values, meant as a drop-in replacement for {@link Command#values()}
     53     * <i>where the array is not modified</i>! This can significantly reduce allocations, as there is no defensive
     54     * array copy.
     55     */
     56    static Command[] getAllValues() {
     57        return CACHED_VALUES;
     58    }
    4859}

  • trunk/src/org/openstreetmap/josm/data/imagery/vectortile/mapbox/CommandInteger.java

    r17994 r18431  
    33
    44import java.util.Arrays;
    5 import java.util.stream.Stream;
    65
    76/**
    … …  
    2221        // Technically, the int is unsigned, but it is easier to work with the long
    2322        final long unsigned = Integer.toUnsignedLong(command);
    24         this.type = Stream.of(Command.values()).filter(e -> e.getId() == (unsigned & 0x7)).findAny()
    25                 .orElseThrow(InvalidMapboxVectorTileException::new);
     23        // By avoiding using a stream for getting the Command type, we go from 72 MB to 13 MB.
     24        // By using a cached value for Command.values(), we further reduce the allocations to 5 MB (new short[] call
     25        // at end of initializer)
     26        Command rType = null;
     27        for (Command tType : Command.getAllValues()) {
     28            if (tType.getId() == (unsigned & 0x7)) {
     29                rType = tType;
     30                break;
     31            }
     32        }
     33        this.type = rType;
     34        if (this.type == null) {
     35            throw new InvalidMapboxVectorTileException();
     36        }
     37
    2638        // This is safe, since we are shifting right 3 when we converted an int to a long (for unsigned).
    2739        // So we <i>cannot</i> lose anything.

  • trunk/src/org/openstreetmap/josm/data/imagery/vectortile/mapbox/Feature.java

    r17994 r18431  
    2525    private static final byte GEOMETRY_TYPE_FIELD = 3;
    2626    private static final byte GEOMETRY_FIELD = 4;
     27    /**
     28     * The number format instance to use (using a static instance gets rid of quite o few allocations)
     29     * Doing this reduced the allocations of {@link #parseTagValue(String, Layer, Number)} from 22.79% of parent to
     30     * 12.2% of parent.
     31     */
     32    private static final NumberFormat NUMBER_FORMAT = NumberFormat.getNumberInstance(Locale.ROOT);
    2733    /**
    2834     * The geometry of the feature. Required.
    … …  
    112118            if (value instanceof Double || value instanceof Float) {
    113119                // reset grouping if the instance is a singleton
    114                 final NumberFormat numberFormat = NumberFormat.getNumberInstance(Locale.ROOT);
    115                 final boolean grouping = numberFormat.isGroupingUsed();
     120
     121                final boolean grouping = NUMBER_FORMAT.isGroupingUsed();
    116122                try {
    117                     numberFormat.setGroupingUsed(false);
    118                     this.tags.put(key, numberFormat.format(value));
     123                    NUMBER_FORMAT.setGroupingUsed(false);
     124                    this.tags.put(key, NUMBER_FORMAT.format(value));
    119125                } finally {
    120                     numberFormat.setGroupingUsed(grouping);
     126                    NUMBER_FORMAT.setGroupingUsed(grouping);
    121127                }
    122128            } else {

  • trunk/src/org/openstreetmap/josm/data/protobuf/ProtobufPacked.java

    r17867 r18431  
    22package org.openstreetmap.josm.data.protobuf;
    33
     4import java.io.ByteArrayOutputStream;
    45import java.util.ArrayList;
    56import java.util.List;
    … …  
    2526        this.bytes = bytes;
    2627        List<Number> numbersT = new ArrayList<>();
     28        // By reusing a ByteArrayOutputStream, we can reduce allocations in nextVarInt from 230 MB to 74 MB.
     29        ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(4);
    2730        while (this.location < bytes.length) {
    28             numbersT.add(ProtobufParser.convertByteArray(this.nextVarInt(), ProtobufParser.VAR_INT_BYTE_SIZE));
     31            numbersT.add(ProtobufParser.convertByteArray(this.nextVarInt(byteArrayOutputStream), ProtobufParser.VAR_INT_BYTE_SIZE));
     32            byteArrayOutputStream.reset();
    2933        }
    3034
    … …  
    4448    }
    4549
    46     private byte[] nextVarInt() {
    47         List<Byte> byteList = new ArrayList<>();
     50    private byte[] nextVarInt(final ByteArrayOutputStream byteArrayOutputStream) {
     51        // In a real world test, the largest List<Byte> seen had 3 elements. Use 4 to avoid most new array allocations.
     52        // Memory allocations went from 368 MB to 280 MB by using an initial array allocation. When using a
     53        // ByteArrayOutputStream, it went down to 230 MB.
    4854        while ((this.bytes[this.location] & ProtobufParser.MOST_SIGNIFICANT_BYTE)
    4955          == ProtobufParser.MOST_SIGNIFICANT_BYTE) {
    5056            // Get rid of the leading bit (shift left 1, then shift right 1 unsigned)
    51             byteList.add((byte) (this.bytes[this.location++] ^ ProtobufParser.MOST_SIGNIFICANT_BYTE));
     57            byteArrayOutputStream.write(this.bytes[this.location++] ^ ProtobufParser.MOST_SIGNIFICANT_BYTE);
    5258        }
    5359        // The last byte doesn't drop the most significant bit
    54         byteList.add(this.bytes[this.location++]);
    55         byte[] byteArray = new byte[byteList.size()];
    56         for (int i = 0; i < byteList.size(); i++) {
    57             byteArray[i] = byteList.get(i);
    58         }
    59 
    60         return byteArray;
     60        byteArrayOutputStream.write(this.bytes[this.location++]);
     61        return byteArrayOutputStream.toByteArray();
    6162    }
    6263}

  • trunk/src/org/openstreetmap/josm/data/protobuf/ProtobufParser.java

    r17867 r18431  
    44import java.io.BufferedInputStream;
    55import java.io.ByteArrayInputStream;
     6import java.io.ByteArrayOutputStream;
    67import java.io.IOException;
    78import java.io.InputStream;
    89import java.util.ArrayList;
    910import java.util.Collection;
    10 import java.util.List;
    1111
    1212import org.openstreetmap.josm.tools.Logging;
    … …  
    156156        this.inputStream.mark(16);
    157157        try {
    158             return WireType.values()[this.inputStream.read() << 3];
     158            return WireType.getAllValues()[this.inputStream.read() << 3];
    159159        } finally {
    160160            this.inputStream.reset();
    … …  
    212212     */
    213213    public byte[] nextVarInt() throws IOException {
    214         List<Byte> byteList = new ArrayList<>();
     214        // Using this reduces the allocations from 150 MB to 95 MB.
     215        final ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(4);
    215216        int currentByte = this.nextByte();
    216217        while ((byte) (currentByte & MOST_SIGNIFICANT_BYTE) == MOST_SIGNIFICANT_BYTE && currentByte > 0) {
    217218            // Get rid of the leading bit (shift left 1, then shift right 1 unsigned)
    218             byteList.add((byte) (currentByte ^ MOST_SIGNIFICANT_BYTE));
     219            byteArrayOutputStream.write((currentByte ^ MOST_SIGNIFICANT_BYTE));
    219220            currentByte = this.nextByte();
    220221        }
    221222        // The last byte doesn't drop the most significant bit
    222         byteList.add((byte) currentByte);
    223         byte[] byteArray = new byte[byteList.size()];
    224         for (int i = 0; i < byteList.size(); i++) {
    225             byteArray[i] = byteList.get(i);
    226         }
    227 
    228         return byteArray;
     223        byteArrayOutputStream.write(currentByte);
     224        return byteArrayOutputStream.toByteArray();
    229225    }
    230226

  • trunk/src/org/openstreetmap/josm/data/protobuf/ProtobufRecord.java

    r17867 r18431  
    44import java.io.IOException;
    55import java.nio.charset.StandardCharsets;
    6 import java.util.stream.Stream;
    76
    87import org.openstreetmap.josm.tools.Utils;
    … …  
    3231        // 7 is 111 (so last three bits)
    3332        byte wireType = (byte) (number.longValue() & 7);
    34         this.type = Stream.of(WireType.values()).filter(wType -> wType.getTypeRepresentation() == wireType).findFirst()
    35           .orElse(WireType.UNKNOWN);
     33        // By not using a stream, we reduce the number of allocations (for getting the WireType) from 257 MB to 40 MB.
     34        // (The remaining 40 MB is from WireType#values). By using the cached getAllValues(), we drop the 40 MB.
     35        WireType tType = null;
     36        for (WireType wType : WireType.getAllValues()) {
     37            if (wType.getTypeRepresentation() == wireType) {
     38                tType = wType;
     39                break;
     40            }
     41        }
     42        this.type = tType;
    3643
    3744        if (this.type == WireType.VARINT) {

  • trunk/src/org/openstreetmap/josm/data/protobuf/WireType.java

    r17867 r18431  
    4545    UNKNOWN(Byte.MAX_VALUE);
    4646
     47    private static final WireType[] CACHED_VALUES = values();
     48
    4749    private final byte type;
    4850
    … …  
    5961        return this.type;
    6062    }
     63
     64    /**
     65     * Get a pre-calculated array of all {@link WireType} values.
     66     * @return An array of values, meant as a drop-in replacement for {@link WireType#values()}
     67     * <i>where the array is not modified</i>! This can significantly reduce allocations, as there is no defensive
     68     * array copy.
     69     */
     70    static WireType[] getAllValues() {
     71        return CACHED_VALUES;
     72    }
    6173}
Note: See TracChangeset for help on using the changeset viewer.