diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java index 2b34157bf3e..9460857bece 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java @@ -772,8 +772,6 @@ private void processMultipolygonRelations() { innerWays.add(way); } else if (member.hasRoleOuter()) { outerWays.add(way); - } else { - LOG.warn("Unexpected role '{}' in multipolygon", member.getRole()); } } processedAreas.add(relation); diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java index a522b471ace..adca987149b 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java @@ -9,6 +9,7 @@ import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.Multimap; import gnu.trove.iterator.TLongIterator; +import gnu.trove.list.array.TDoubleArrayList; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -372,7 +373,7 @@ private void buildBasicGraph( IntersectionVertex fromVertex = null; IntersectionVertex toVertex = null; - ArrayList segmentCoordinates = new ArrayList<>(); + TDoubleArrayList segmentCoordinates = new TDoubleArrayList(100); /* * Traverse through all the nodes of this edge. For nodes which are not shared with any other edge, do not create endpoints -- just @@ -412,7 +413,8 @@ private void buildBasicGraph( * the only processing we do on other nodes is to accumulate their geometry */ if (segmentCoordinates.isEmpty()) { - segmentCoordinates.add(osmStartNode.getCoordinate()); + segmentCoordinates.add(osmStartNode.lon); + segmentCoordinates.add(osmStartNode.lat); } if ( @@ -425,14 +427,14 @@ private void buildBasicGraph( osmEndNode.isEntrance() || vertexGenerator.nodesInBarrierWays().containsKey(osmEndNode) ) { - segmentCoordinates.add(osmEndNode.getCoordinate()); + segmentCoordinates.add(osmEndNode.lon); + segmentCoordinates.add(osmEndNode.lat); - geometry = GeometryUtils.getGeometryFactory().createLineString( - segmentCoordinates.toArray(new Coordinate[0]) - ); + geometry = GeometryUtils.makeLineString(segmentCoordinates.toArray()); segmentCoordinates.clear(); } else { - segmentCoordinates.add(osmEndNode.getCoordinate()); + segmentCoordinates.add(osmEndNode.lon); + segmentCoordinates.add(osmEndNode.lat); continue; } @@ -554,28 +556,23 @@ private void buildBarrierEdges(VertexGenerator vertexGenerator) { private Optional getPlatform(OsmDatabase osmdb, OsmWay way) { var references = way.getMultiTagValues(params.boardingAreaRefTags()); - if (way.isBoardingLocation() && !references.isEmpty()) { - var nodeRefs = way.getNodeRefs(); - var size = nodeRefs.size(); - var nodes = new Coordinate[size]; - for (int i = 0; i < size; i++) { - nodes[i] = osmdb.getNode(nodeRefs.get(i)).getCoordinate(); - } + if (!way.isBoardingLocation() || references.isEmpty()) { + return Optional.empty(); + } + var nodeRefs = way.getNodeRefs(); + var size = nodeRefs.size(); + var nodes = new Coordinate[size]; + for (int i = 0; i < size; i++) { + nodes[i] = osmdb.getNode(nodeRefs.get(i)).getCoordinate(); + } - var geometryFactory = GeometryUtils.getGeometryFactory(); + var geometryFactory = GeometryUtils.getGeometryFactory(); - var geometry = geometryFactory.createLineString(nodes); + var geometry = geometryFactory.createLineString(nodes); - return Optional.of( - new Platform( - params.edgeNamer().getName(way, "platform " + way.getId()), - geometry, - references - ) - ); - } else { - return Optional.empty(); - } + return Optional.of( + new Platform(params.edgeNamer().getName(way, "platform " + way.getId()), geometry, references) + ); } private void validateBarriers() { diff --git a/application/src/main/java/org/opentripplanner/osm/model/OsmEntity.java b/application/src/main/java/org/opentripplanner/osm/model/OsmEntity.java index 1999f0442d5..b00a2d64f51 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmEntity.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmEntity.java @@ -10,9 +10,9 @@ import java.time.Duration; import java.time.format.DateTimeParseException; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -21,7 +21,6 @@ import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; -import java.util.stream.Collectors; import javax.annotation.Nullable; import org.opentripplanner.core.model.accessibility.Accessibility; import org.opentripplanner.core.model.i18n.I18NString; @@ -37,6 +36,7 @@ */ public abstract class OsmEntity { + private static final Pattern I18N_PATTERN = Pattern.compile("\\{(.*?)}"); /** * highway=* values that we don't want to even consider when building the graph. */ @@ -133,6 +133,24 @@ public abstract class OsmEntity { */ protected static final Set CHECKED_MODES = Set.of("foot", "bicycle", "motorcar"); + private static final Set WALK_ONLY_HIGHWAY_VALUES = Set.of("footway", "step", "corridor"); + private static final Set NO_ACCESS_VALUES = Set.of("no", "license", "dismount"); + private static final Set HIGHWAY_BOARDING_LOCATION_VALUES = Set.of( + "platform", + "bus_stop" + ); + private static final Set RAILWAY_BOARDING_LOCATION_VALUES = Set.of( + "tram_stop", + "station", + "halt" + ); + private static final Set AMENITY_BOARDING_LOCATION_VALUES = Set.of( + "bus_station", + "amenity", + "ferry_terminal" + ); + private static final Set RAILWAY_PLATFORM_VALUES = Set.of("platform", "platform_edge"); + /** * Mapping for the fallback key for checking access restrictions for each access mode in OSM * However, access is not included because we are skeptical of access=yes tags. @@ -145,8 +163,6 @@ public abstract class OsmEntity { "bicycle", "vehicle" ); - private static final Set WALK_ONLY_HIGHWAYS = Set.of("footway", "step", "corridor"); - private static final Set NO_ACCESS_TAGS = Set.of("no", "license", "dismount"); private static final Map OSM_TAGS_FOR_TRAVERSAL_PERMISSION = Map.of( StreetTraversalPermission.CAR, @@ -157,6 +173,11 @@ public abstract class OsmEntity { "foot" ); + /// This is nullable for performance reasons. + /// + /// You could use an empty map, but using null allows you to skip the lower-casing and hash lookup + /// per tag, which is the hottest path during OSM processing. + @Nullable private final Map tags; protected final long id; @@ -175,7 +196,7 @@ public abstract class OsmEntity { /** * Constructor for immutable OsmEntity */ - protected OsmEntity(long id, Map tags, OsmProvider osmProvider) { + protected OsmEntity(long id, @Nullable Map tags, OsmProvider osmProvider) { this.id = id; // calling Map.copyOf here costs about 10% of parsing performance, so we use // Collections.unmodifiableMap in the getter @@ -202,20 +223,31 @@ public long getId() { * The tags of an entity (immutable). */ public Map getTags() { - return Collections.unmodifiableMap(tags); + if (this.isTagless()) { + return Map.of(); + } else { + return Collections.unmodifiableMap(tags); + } } /** * Is the tag defined? */ - public boolean hasTag(String tag) { - return getTag(tag) != null; + public final boolean hasTag(String tag) { + return !this.isTagless() && getTag(tag) != null; + } + + /** + * Does the entity contain any tags at all? + */ + final boolean isTagless() { + return this.tags == null; } /** * Determines if a tag contains a false value. 'no', 'false', and '0' are considered false. */ - public boolean isTagFalse(String tag) { + public final boolean isTagFalse(String tag) { return isFalse(getTag(tag)); } @@ -235,17 +267,10 @@ public Accessibility explicitWheelchairAccessibility() { /** * Determines if a tag contains a true value. 'yes', 'true', and '1' are considered true. */ - public boolean isTagTrue(String tag) { + public final boolean isTagTrue(String tag) { return isTrue(getTag(tag)); } - /** - * Returns true if bicycle dismounts are forced. - */ - public boolean isBicycleDismountForced() { - return isTag("bicycle", "dismount"); - } - public boolean isSidewalk() { return isTag("footway", "sidewalk") && isTag("highway", "footway"); } @@ -270,6 +295,9 @@ protected Optional checkModePermission(String mode) { * or a parent mode, either with a directional suffix or not, empty if it is not specified. */ protected Optional checkModePermission(String mode, TraverseDirection direction) { + if (isTagless()) { + return Optional.empty(); + } // check if the exact directional tag allows or denies access if (direction != DIRECTIONLESS) { if (isExplicitlyAllowed(mode + direction.tagSuffix())) { @@ -309,7 +337,10 @@ protected boolean isExplicitlyAllowed(String key) { * Returns null if tag is not present. */ @Nullable - public String getTag(String tag) { + public final String getTag(String tag) { + if (this.isTagless()) { + return null; + } tag = tag.toLowerCase(); return tags.get(tag); } @@ -470,8 +501,8 @@ public OptionalInt parseIntOrBoolean(String tag, Consumer errorHandler) /** * Checks if a tag contains the specified value. */ - public boolean isTag(String tag, String value) { - return value != null && value.equals(tags.get(tag.toLowerCase())); + public final boolean isTag(String tag, String value) { + return !isTagless() && value != null && value.equals(tags.get(tag.toLowerCase())); } /** @@ -492,7 +523,7 @@ public boolean isOneOfTags(String key, Set oneOfTags) { */ @Nullable public I18NString getAssumedName() { - if (tags.containsKey("name")) { + if (hasTag("name")) { return TranslatedString.getDeduplicatedI18NString( this.generateI18NForPattern("{name}"), false @@ -506,8 +537,9 @@ public I18NString getAssumedName() { if (creativeName != null) { return this.creativeName; } - if (tags.containsKey("ref")) { - return new NonLocalizedString(tags.get("ref")); + var ref = getTag("ref"); + if (ref != null) { + return new NonLocalizedString(ref); } return null; } @@ -524,7 +556,7 @@ public I18NString getAssumedName() { public Map generateI18NForPattern(String pattern) { Map i18n = new HashMap<>(); i18n.put(null, new StringBuffer()); - Matcher matcher = Pattern.compile("\\{(.*?)}").matcher(pattern); + Matcher matcher = I18N_PATTERN.matcher(pattern); int lastEnd = 0; while (matcher.find()) { @@ -621,7 +653,7 @@ public Optional isOneWay(@Nullable String mode) { return Optional.empty(); } - if ("foot".equals(mode) && !isOneOfTags("highway", WALK_ONLY_HIGHWAYS)) { + if ("foot".equals(mode) && !isOneOfTags("highway", WALK_ONLY_HIGHWAY_VALUES)) { return Optional.empty(); } @@ -705,14 +737,14 @@ public boolean isParkAndRide() { * @return whether the node is a place used to board a public transport vehicle */ public boolean isBoardingLocation() { + // the majority of nodes have no tags at all, so this yields a good speed-up + if (isTagless()) { + return false; + } return ( - isTag("highway", "bus_stop") || - isTag("railway", "tram_stop") || - isTag("railway", "station") || - isTag("railway", "halt") || - isTag("amenity", "bus_station") || - isTag("amenity", "ferry_terminal") || - isTag("highway", "platform") || + isOneOfTags("highway", HIGHWAY_BOARDING_LOCATION_VALUES) || + isOneOfTags("railway", RAILWAY_BOARDING_LOCATION_VALUES) || + isOneOfTags("amenity", AMENITY_BOARDING_LOCATION_VALUES) || isPlatform() ); } @@ -726,9 +758,7 @@ public boolean isBoardingLocation() { **/ public boolean isPlatform() { var isPlatform = - isTag("public_transport", "platform") || - isTag("railway", "platform") || - isTag("railway", "platform_edge"); + isTag("public_transport", "platform") || isOneOfTags("railway", RAILWAY_PLATFORM_VALUES); return isPlatform && !isTag("usage", "tourism"); } @@ -775,14 +805,21 @@ public String url() { * Values are split by semicolons. */ public Set getMultiTagValues(Set refTags) { - return refTags - .stream() - .map(this::getTag) - .filter(Objects::nonNull) - .flatMap(v -> Arrays.stream(v.split(";"))) - .map(String::strip) - .filter(v -> !v.isBlank()) - .collect(Collectors.toUnmodifiableSet()); + // we try to keep the allocations low, so only allocate a small hash set by default + Set result = HashSet.newHashSet(2); + for (var tag : refTags) { + var value = getTag(tag); + if (value == null) { + continue; + } + for (var part : value.split(";")) { + var stripped = part.strip(); + if (!stripped.isBlank()) { + result.add(stripped); + } + } + } + return result; } public OsmProvider getOsmProvider() { @@ -842,10 +879,11 @@ public boolean isElevator() { * of other information. */ public boolean isWheelchairAccessible() { - if (isTagTrue("wheelchair")) { + var wheelchairValue = getTag("wheelchair"); + if (isTrue(wheelchairValue)) { return true; } - if (isTagFalse("wheelchair")) { + if (isFalse(wheelchairValue)) { return false; } if (isOneOfTags("barrier", WHEELCHAIR_INACCESSIBLE_BARRIERS)) { @@ -891,7 +929,7 @@ public boolean isExplicitlyUnnamed() { * Returns true if this tag is explicitly access to this entity. */ private boolean isExplicitlyDenied(String key) { - return isOneOfTags(key, NO_ACCESS_TAGS); + return isOneOfTags(key, NO_ACCESS_VALUES); } public StreetTraversalPermission getPermission() { diff --git a/application/src/main/java/org/opentripplanner/osm/model/OsmNode.java b/application/src/main/java/org/opentripplanner/osm/model/OsmNode.java index 6d4e48ba138..7a4af6b4b51 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmNode.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmNode.java @@ -38,11 +38,11 @@ public Coordinate getCoordinate() { } public boolean hasHighwayTrafficLight() { - return hasTag("highway") && "traffic_signals".equals(getTag("highway")); + return "traffic_signals".equals(getTag("highway")); } public boolean hasCrossingTrafficLight() { - return hasTag("crossing") && "traffic_signals".equals(getTag("crossing")); + return "traffic_signals".equals(getTag("crossing")); } /** @@ -51,6 +51,10 @@ public boolean hasCrossingTrafficLight() { * @return true if it does */ public boolean isBarrier() { + // the majority of nodes have no tags at all, so this yields a good speed-up + if (this.isTagless()) { + return false; + } return overridePermissions(ALL) != ALL; } @@ -69,6 +73,10 @@ public boolean isStationEntrance() { * @return True if this entity provides an entrance to a platform or similar entity */ public boolean isEntrance() { + // the majority of nodes have no tags at all, so this yields a good speed-up + if (this.isTagless()) { + return false; + } return ( (isStationEntrance() || isTag("entrance", "yes") || isTag("entrance", "main")) && !isTag("access", "private") && diff --git a/application/src/main/java/org/opentripplanner/osm/model/OsmNodeBuilder.java b/application/src/main/java/org/opentripplanner/osm/model/OsmNodeBuilder.java index a55cb3dd2c1..9deb5889552 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmNodeBuilder.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmNodeBuilder.java @@ -6,11 +6,12 @@ public class OsmNodeBuilder { - private static final Map EMPTY_TAGS = Map.of(); + private static final Map EMPTY_TAGS = null; private long id; private double lat; private double lon; - // many nodes don't have any tags so we start with an empty immutable map + // the vast majority of nodes don't have any tags, so we start with null, because that has no + // allocations at all private Map tags = EMPTY_TAGS; private OsmProvider osmProvider; diff --git a/application/src/main/java/org/opentripplanner/osm/wayproperty/specifier/Condition.java b/application/src/main/java/org/opentripplanner/osm/wayproperty/specifier/Condition.java index ed8a856e600..8bee22bab8e 100644 --- a/application/src/main/java/org/opentripplanner/osm/wayproperty/specifier/Condition.java +++ b/application/src/main/java/org/opentripplanner/osm/wayproperty/specifier/Condition.java @@ -133,7 +133,7 @@ public String toString() { record Equals(String key, String value) implements Condition { @Override public boolean isExtendedKeyMatch(OsmEntity way, String exKey) { - return way.hasTag(exKey) && way.isTag(exKey, value); + return way.isTag(exKey, value); } @Override diff --git a/application/src/main/java/org/opentripplanner/osm/wayproperty/specifier/ExactMatchSpecifier.java b/application/src/main/java/org/opentripplanner/osm/wayproperty/specifier/ExactMatchSpecifier.java index 64c95c1149e..221c8e98d3c 100644 --- a/application/src/main/java/org/opentripplanner/osm/wayproperty/specifier/ExactMatchSpecifier.java +++ b/application/src/main/java/org/opentripplanner/osm/wayproperty/specifier/ExactMatchSpecifier.java @@ -1,7 +1,6 @@ package org.opentripplanner.osm.wayproperty.specifier; import java.util.Arrays; -import java.util.List; import java.util.stream.Collectors; import org.opentripplanner.osm.model.OsmEntity; import org.opentripplanner.osm.model.TraverseDirection; @@ -27,7 +26,7 @@ public class ExactMatchSpecifier implements OsmSpecifier { */ public static final int MATCH_MULTIPLIER = 200; public static final int NO_MATCH_SCORE = 0; - private final List conditions; + private final Condition[] conditions; private final int bestMatchScore; public ExactMatchSpecifier(String spec) { @@ -35,8 +34,8 @@ public ExactMatchSpecifier(String spec) { } public ExactMatchSpecifier(Condition... conditions) { - this.conditions = Arrays.asList(conditions); - bestMatchScore = this.conditions.size() * MATCH_MULTIPLIER; + this.conditions = conditions; + bestMatchScore = this.conditions.length * MATCH_MULTIPLIER; } @Override @@ -46,19 +45,34 @@ public int matchScore(OsmEntity way, TraverseDirection direction) { @Override public String toDocString() { - return conditions.stream().map(Object::toString).collect(Collectors.joining("; ")); + return Arrays.stream(conditions).map(Object::toString).collect(Collectors.joining("; ")); } public boolean allTagsMatch(OsmEntity way) { - return conditions.stream().allMatch(o -> o.isMatch(way)); + for (var c : conditions) { + if (!c.isMatch(way)) { + return false; + } + } + return true; } public boolean allBackwardTagsMatch(OsmEntity way) { - return conditions.stream().allMatch(c -> c.isBackwardMatch(way)); + for (var c : conditions) { + if (!c.isBackwardMatch(way)) { + return false; + } + } + return true; } public boolean allForwardTagsMatch(OsmEntity way) { - return conditions.stream().allMatch(c -> c.isForwardMatch(way)); + for (var c : conditions) { + if (!c.isForwardMatch(way)) { + return false; + } + } + return true; } private boolean allTagsMatch(OsmEntity way, TraverseDirection direction) { diff --git a/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java b/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java index 870a313c74c..151f0deb1ef 100644 --- a/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java +++ b/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java @@ -22,15 +22,6 @@ void lowerCaseKeys() { assertEquals("baz", entity.getTag("foo")); } - @Test - void testIsBicycleDismountForced() { - OsmWay way = OsmWay.of().build(); - assertFalse(way.isBicycleDismountForced()); - - way = OsmWay.of().withTag("bicycle", "dismount").build(); - assertTrue(way.isBicycleDismountForced()); - } - @Test void testAreaMustContain3Nodes() { OsmWay way = OsmWay.of().withTag("area", "yes").build(); diff --git a/domain-core/src/main/java/org/opentripplanner/core/model/i18n/TranslatedString.java b/domain-core/src/main/java/org/opentripplanner/core/model/i18n/TranslatedString.java index bcb05dc8b98..4986eb9eddd 100644 --- a/domain-core/src/main/java/org/opentripplanner/core/model/i18n/TranslatedString.java +++ b/domain-core/src/main/java/org/opentripplanner/core/model/i18n/TranslatedString.java @@ -114,8 +114,9 @@ static I18NString getI18NString( if (translations.isEmpty()) { throw new IllegalArgumentException("At least one translation must be provided"); } - if (TRANSLATION_CACHE.containsKey(translations)) { - return TRANSLATION_CACHE.get(translations); + var t = TRANSLATION_CACHE.get(translations); + if (t != null) { + return t; } else { I18NString ret; // Check if we only have one name, even under multiple languages