Changeset 18478 in josm


Ignore:
Timestamp:
2022-06-08T22:20:53+02:00 (23 months ago)
Author:
taylor.smock
Message:

Really fix failing tests in VectorDataSetTest

The test failed ~50% of the time (it depended upon the HashSet ordering).
We now use an ArrayList instead, and a the optimization that caused the
failure is not as significant anymore (node search is significantly cheaper
since r18465).

Location:
trunk
Files:
3 edited

Legend:

Unmodified
Added
Removed

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

    r18473 r18478  
    55import java.io.IOException;
    66import java.io.InputStream;
     7import java.util.ArrayList;
    78import java.util.Collection;
    8 import java.util.HashSet;
    99import java.util.List;
    1010import java.util.Objects;
    … …  
    5353            ProtobufParser parser = new ProtobufParser(inputStream);
    5454            Collection<ProtobufRecord> protobufRecords = parser.allRecords();
    55             this.layers = new HashSet<>(protobufRecords.size());
     55            this.layers = new ArrayList<>(protobufRecords.size());
    5656            for (ProtobufRecord protoBufRecord : protobufRecords) {
    5757                if (protoBufRecord.getField() == Layer.LAYER_FIELD) {
    … …  
    6666                }
    6767            }
    68             this.layers = new HashSet<>(this.layers);
     68            this.layers = new ArrayList<>(this.layers);
    6969
    7070            this.extent = layers.stream().filter(Objects::nonNull).mapToInt(Layer::getExtent).max().orElse(Layer.DEFAULT_EXTENT);

  • trunk/src/org/openstreetmap/josm/data/vector/VectorDataStore.java

    r18477 r18478  
    1313import java.util.Collection;
    1414import java.util.Collections;
    15 import java.util.HashMap;
    1615import java.util.List;
    1716import java.util.Map;
    … …  
    2221import org.openstreetmap.gui.jmapviewer.interfaces.ICoordinate;
    2322import org.openstreetmap.josm.data.IQuadBucketType;
    24 import org.openstreetmap.josm.data.coor.ILatLon;
    2523import org.openstreetmap.josm.data.coor.LatLon;
    2624import org.openstreetmap.josm.data.imagery.vectortile.VectorTile;
    … …  
    163161
    164162    private synchronized <T extends Tile & VectorTile> VectorNode pointToNode(T tile, Layer layer,
    165       Collection<VectorPrimitive> featureObjects, int x, int y, final Map<ILatLon, VectorNode> nodeMap) {
     163      Collection<VectorPrimitive> featureObjects, int x, int y) {
    166164        final BBox tileBbox;
    167165        if (tile instanceof IQuadBucketType) {
    … …  
    179177                tileBbox.getMinLon() + (tileBbox.getMaxLon() - tileBbox.getMinLon()) * x / layerExtent
    180178        );
    181         if (nodeMap.containsKey(coords)) {
    182             return nodeMap.get(coords);
    183         }
    184179        final Collection<VectorNode> nodes = this.store
    185180          .searchNodes(new BBox(coords.lon(), coords.lat(), VectorDataSet.DUPE_NODE_DISTANCE));
    … …  
    212207        node.setCoor(coords);
    213208        featureObjects.add(node);
    214         nodeMap.put(node.getCoor(), node);
    215209        return node;
    216210    }
    217211
    218212    private <T extends Tile & VectorTile> List<VectorWay> pathToWay(T tile, Layer layer,
    219       Collection<VectorPrimitive> featureObjects, Path2D shape, Map<ILatLon, VectorNode> nodeMap) {
     213      Collection<VectorPrimitive> featureObjects, Path2D shape) {
    220214        final PathIterator pathIterator = shape.getPathIterator(null);
    221215        final List<VectorWay> ways = new ArrayList<>(
    222                 Utils.filteredCollection(pathIteratorToObjects(tile, layer, featureObjects, pathIterator, nodeMap), VectorWay.class));
     216                Utils.filteredCollection(pathIteratorToObjects(tile, layer, featureObjects, pathIterator), VectorWay.class));
    223217        // These nodes technically do not exist, so we shouldn't show them
    224218        for (VectorWay way : ways) {
    … …  
    234228
    235229    private <T extends Tile & VectorTile> List<VectorPrimitive> pathIteratorToObjects(T tile, Layer layer,
    236       Collection<VectorPrimitive> featureObjects, PathIterator pathIterator, Map<ILatLon, VectorNode> nodeMap) {
     230      Collection<VectorPrimitive> featureObjects, PathIterator pathIterator) {
    237231        final List<VectorNode> nodes = new ArrayList<>();
    238232        final double[] coords = new double[6];
    … …  
    255249            }
    256250            if (PathIterator.SEG_MOVETO == type || PathIterator.SEG_LINETO == type) {
    257                 final VectorNode node = pointToNode(tile, layer, featureObjects, (int) coords[0], (int) coords[1], nodeMap);
     251                final VectorNode node = pointToNode(tile, layer, featureObjects, (int) coords[0], (int) coords[1]);
    258252                nodes.add(node);
    259253            } else if (PathIterator.SEG_CLOSE != type) {
    … …  
    272266
    273267    private <T extends Tile & VectorTile> VectorRelation areaToRelation(T tile, Layer layer,
    274       Collection<VectorPrimitive> featureObjects, Area area, Map<ILatLon, VectorNode> nodeMap) {
     268      Collection<VectorPrimitive> featureObjects, Area area) {
    275269        VectorRelation vectorRelation = new VectorRelation(layer.getName());
    276         for (VectorPrimitive member : pathIteratorToObjects(tile, layer, featureObjects, area.getPathIterator(null), nodeMap)) {
     270        for (VectorPrimitive member : pathIteratorToObjects(tile, layer, featureObjects, area.getPathIterator(null))) {
    277271            final String role;
    278272            if (member instanceof VectorWay && ((VectorWay) member).isClosed()) {
    … …  
    294288        // Using a map reduces the cost of addFeatureData from 2,715,158,632 bytes to 235,042,184 bytes (-91.3%)
    295289        // This was somewhat variant, with some runs being closer to ~560 MB (still -80%).
    296         final Map<ILatLon, VectorNode> nodeMap = new HashMap<>();
    297290        for (Layer layer : tile.getLayers()) {
    298291            Map<GeometryTypes, List<Feature>> grouped = layer.getFeatures().stream().collect(Collectors.groupingBy(Feature::getGeometryType));
    … …  
    300293            for (GeometryTypes type : GeometryTypes.values()) {
    301294                if (grouped.containsKey(type)) {
    302                     addFeatureData(tile, layer, grouped.get(type), nodeMap);
     295                    addFeatureData(tile, layer, grouped.get(type));
    303296                }
    304297            }
    … …  
    313306    }
    314307
    315     private <T extends Tile & VectorTile> void addFeatureData(T tile, Layer layer, Collection<Feature> features, Map<ILatLon, VectorNode> nodeMap) {
     308    private <T extends Tile & VectorTile> void addFeatureData(T tile, Layer layer, Collection<Feature> features) {
    316309        for (Feature feature : features) {
    317310            try {
    318                 addFeatureData(tile, layer, feature, nodeMap);
     311                addFeatureData(tile, layer, feature);
    319312            } catch (IllegalArgumentException e) {
    320313                Logging.error("Cannot add vector data for feature {0} of tile {1}: {2}", feature, tile, e.getMessage());
    … …  
    324317    }
    325318
    326     private <T extends Tile & VectorTile> void addFeatureData(T tile, Layer layer, Feature feature, Map<ILatLon, VectorNode> nodeMap) {
     319    private <T extends Tile & VectorTile> void addFeatureData(T tile, Layer layer, Feature feature) {
    327320        // This will typically be larger than primaryFeatureObjects, but this at least avoids quite a few ArrayList#grow calls
    328321        List<VectorPrimitive> featureObjects = new ArrayList<>(feature.getGeometryObject().getShapes().size());
    329322        List<VectorPrimitive> primaryFeatureObjects = new ArrayList<>(feature.getGeometryObject().getShapes().size());
    330323        for (Shape shape : feature.getGeometryObject().getShapes()) {
    331             primaryFeatureObjects.add(shapeToPrimaryFeatureObject(tile, layer, shape, featureObjects, nodeMap));
     324            primaryFeatureObjects.add(shapeToPrimaryFeatureObject(tile, layer, shape, featureObjects));
    332325        }
    333326        final VectorPrimitive primitive;
    … …  
    374367
    375368    private <T extends Tile & VectorTile> VectorPrimitive shapeToPrimaryFeatureObject(
    376             T tile, Layer layer, Shape shape, List<VectorPrimitive> featureObjects, Map<ILatLon, VectorNode> nodeMap) {
     369            T tile, Layer layer, Shape shape, List<VectorPrimitive> featureObjects) {
    377370        final VectorPrimitive primitive;
    378371        if (shape instanceof Ellipse2D) {
    379372            primitive = pointToNode(tile, layer, featureObjects,
    380                     (int) ((Ellipse2D) shape).getCenterX(), (int) ((Ellipse2D) shape).getCenterY(), nodeMap);
     373                    (int) ((Ellipse2D) shape).getCenterX(), (int) ((Ellipse2D) shape).getCenterY());
    381374        } else if (shape instanceof Path2D) {
    382             primitive = pathToWay(tile, layer, featureObjects, (Path2D) shape, nodeMap).stream().findFirst().orElse(null);
     375            primitive = pathToWay(tile, layer, featureObjects, (Path2D) shape).stream().findFirst().orElse(null);
    383376        } else if (shape instanceof Area) {
    384             primitive = areaToRelation(tile, layer, featureObjects, (Area) shape, nodeMap);
     377            primitive = areaToRelation(tile, layer, featureObjects, (Area) shape);
    385378            primitive.put(RELATION_TYPE, MULTIPOLYGON_TYPE);
    386379        } else {

  • trunk/test/unit/org/openstreetmap/josm/data/vector/VectorDataSetTest.java

    r18477 r18478  
    1414import org.awaitility.Durations;
    1515import org.junit.jupiter.api.BeforeEach;
    16 import org.junit.jupiter.api.Test;
     16import org.junit.jupiter.api.RepeatedTest;
    1717import org.junit.jupiter.api.extension.RegisterExtension;
    1818import org.openstreetmap.josm.TestUtils;
    … …  
    9999    }
    100100
    101     @Test
     101    /**
     102     * This failed about 1/2 times.
     103     * It tests for node deduplication.
     104     */
     105    @RepeatedTest(10)
    102106    void testNodeDeduplication() {
    103107        final VectorDataSet dataSet = this.layer.getData();
Note: See TracChangeset for help on using the changeset viewer.