Skip to content

Commit

Permalink
Significantly improve performance for StreetAddressTest
Browse files Browse the repository at this point in the history
This is mostly done by reducing the amount of garbage collection that
needs to occur.

In addition, update Wiremock for tests.

Signed-off-by: Taylor Smock <tsmock@meta.com>
  • Loading branch information
tsmock committed Sep 26, 2024
1 parent cecb9f4 commit 29d5461
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 17 deletions.
4 changes: 2 additions & 2 deletions gradle.properties
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# The minimum JOSM version this plugin is compatible with (can be any numeric version
plugin.main.version = 18877
plugin.main.version = 19067
# The JOSM version this plugin is currently compiled against
# Please make sure this version is available at https://josm.openstreetmap.de/download
# The special values "latest" and "tested" are also possible here, but not recommended.
plugin.compile.version = 18877
plugin.compile.version = 19067
plugin.canloadatruntime = true
plugin.author = Taylor Smock
plugin.class = org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin
Expand Down
143 changes: 143 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
<project xmlns="http://maven.apache.org/POM/4.0.0">
<modelVersion>4.0.0</modelVersion>
<groupId>org.openstreetmap.josm.plugins</groupId>
<artifactId>MapWithAI</artifactId>
<version>SNAPSHOT</version>
<properties>
<jmockit.version>1.49.a</jmockit.version>
<pmd.version>7.5.0</pmd.version>
<jacoco.version>0.8.12</jacoco.version>
<checkstyle.version>10.18.1</checkstyle.version>
<spotbugs.version>4.8.6</spotbugs.version>
</properties>
<repositories>
<repository>
<id>JOSM-releases</id>
<url>https://josm.openstreetmap.de/nexus/content/repositories/releases/</url>
</repository>
<repository>
<id>JOSM-snapshots</id>
<url>https://josm.openstreetmap.de/nexus/content/repositories/snapshots/</url>
</repository>
</repositories>
<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.junit</groupId>
<artifactId>junit-bom</artifactId>
<version>5.11.1</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>
<dependencies>
<dependency>
<groupId>org.openstreetmap.josm</groupId>
<artifactId>josm</artifactId>
<version>19067</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.openstreetmap.josm.plugins</groupId>
<artifactId>pmtiles</artifactId>
<version>SNAPSHOT</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.openstreetmap.josm.plugins</groupId>
<artifactId>utilsplugin2</artifactId>
<version>SNAPSHOT</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.openstreetmap.josm</groupId>
<artifactId>josm-unittest</artifactId>
<version>SNAPSHOT</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.wiremock</groupId>
<artifactId>wiremock</artifactId>
<version>3.9.1</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-params</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jmockit</groupId>
<artifactId>jmockit</artifactId>
<version>1.49.a</version>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.13.0</version>
<configuration>
<release>17</release>
</configuration>
</plugin>
<plugin>
<groupId>com.diffplug.spotless</groupId>
<artifactId>spotless-maven-plugin</artifactId>
<version>2.43.0</version>
<configuration>
<java>
<eclipse>
<version>4.21</version>
<file>${project.basedir}/../00_core_tools/eclipse/formatter.xml</file>
</eclipse>
<removeUnusedImports/>
<licenseHeader>
<content>// License: GPL. For details, see LICENSE file.</content>
</licenseHeader>
<includes>
<include>src/main/java/**/*.java</include>
<include>src/test/unit/**/*.java</include>
<include>src/test/integration/**/*.java</include>
</includes>
</java>
</configuration>
<executions>
<execution>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<version>3.5.0</version>
<executions>
<execution>
<id>enforce-maven</id>
<goals>
<goal>enforce</goal>
</goals>
</execution>
</executions>
<configuration>
<rules>
<requireMavenVersion>
<version>3.6.3</version>
</requireMavenVersion>
</rules>
</configuration>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ public class StreetAddressTest extends Test {
public static final double BBOX_EXPANSION = 0.002;
private static final String ADDR_STREET = "addr:street";
private final Set<OsmPrimitive> namePrimitiveMap = new HashSet<>();
private final Map<Point2D, Set<String>> nameMap = new HashMap<>();
private final Map<Point2D, List<OsmPrimitive>> primitiveCellMap = new HashMap<>();
private final Map<Point, Set<String>> nameMap = new HashMap<>();
private final Map<Point, List<OsmPrimitive>> primitiveCellMap = new HashMap<>();
/**
* Classified highways. This uses a {@link Set} instead of a {@link List} since
* the MapWithAI code doesn't care about order.
Expand Down Expand Up @@ -146,7 +146,8 @@ private void realVisit(OsmPrimitive primitive) {
final var n1 = nodes.get(i);
final var n2 = nodes.get(i + 1);
for (Point2D cell : ValUtil.getSegmentCells(n1, n2, gridDetail)) {
this.nameMap.computeIfAbsent(cell, k -> new HashSet<>()).addAll(names);
this.nameMap.computeIfAbsent(new Point(cell.getX(), cell.getY()), k -> new HashSet<>())
.addAll(names);
}
}
} else if (hasStreetAddressTags(primitive) && !primitive.isOutsideDownloadArea()) {
Expand All @@ -159,7 +160,7 @@ private void realVisit(OsmPrimitive primitive) {
if (en != null) {
long x = (long) Math.floor(en.getX() * gridDetail);
long y = (long) Math.floor(en.getY() * gridDetail);
final var point = new Point2D.Double(x, y);
final var point = new Point(x, y);
primitiveCellMap.computeIfAbsent(point, p -> new ArrayList<>()).add(primitive);
}
}
Expand All @@ -174,7 +175,7 @@ private static Collection<String> getWayNames(IPrimitive way) {
.filter(s -> !s.isEmpty()).collect(Collectors.toSet());
}

private Collection<String> getSurroundingHighwayNames(Point2D point2D) {
private Collection<String> getSurroundingHighwayNames(Point point2D) {
if (this.nameMap.isEmpty()) {
return Collections.emptySet();
}
Expand All @@ -183,8 +184,9 @@ private Collection<String> getSurroundingHighwayNames(Point2D point2D) {
while (surroundingWays.isEmpty()) {
for (int x = -surrounding; x <= surrounding; x++) {
for (int y = -surrounding; y <= surrounding; y++) {
final var key = new Point2D.Double((long) Math.floor(point2D.getX() + x),
(long) Math.floor(point2D.getY() + y));
final var xPoint = (long) Math.floor(point2D.x() + x);
final var yPoint = (long) Math.floor(point2D.y() + y);
final var key = new Point(xPoint, yPoint);
if (this.nameMap.containsKey(key)) {
surroundingWays.addAll(this.nameMap.get(key));
}
Expand Down Expand Up @@ -303,4 +305,20 @@ public static BBox expandBBox(BBox bbox, double degree) {
bbox.add(bbox.getTopLeftLon() - degree, bbox.getTopLeftLat() + degree);
return bbox;
}
}

private record Point(double x, double y) implements Comparable<Point> {

@Override
public int compareTo(Point other) {
if (other.x == this.x && other.y == this.y) {
return 0;
}
if (other.x < this.x) {
return -1;
}
if (other.x > this.x) {
return 1;
}
return Double.compare(other.y, this.y);
}
}}
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,9 @@

import com.github.tomakehurst.wiremock.WireMockServer;
import com.github.tomakehurst.wiremock.client.WireMock;
import com.github.tomakehurst.wiremock.common.FileSource;
import com.github.tomakehurst.wiremock.extension.Parameters;
import com.github.tomakehurst.wiremock.extension.ResponseTransformer;
import com.github.tomakehurst.wiremock.http.Request;
import com.github.tomakehurst.wiremock.extension.ResponseTransformerV2;
import com.github.tomakehurst.wiremock.http.Response;
import com.github.tomakehurst.wiremock.stubbing.ServeEvent;

/**
* Test annotation to ensure that wiremock is used
Expand All @@ -59,7 +57,7 @@
*
* @author Taylor Smock
*/
class WireMockUrlTransformer extends ResponseTransformer {
class WireMockUrlTransformer implements ResponseTransformerV2 {
private final ExtensionContext context;

public WireMockUrlTransformer(ExtensionContext context) {
Expand All @@ -72,8 +70,8 @@ public String getName() {
}

@Override
public Response transform(Request request, Response response, FileSource files, Parameters parameters) {
if (!request.getUrl().endsWith("/capabilities")
public Response transform(Response response, ServeEvent serveEvent) {
if (!serveEvent.getRequest().getUrl().endsWith("/capabilities")
&& response.getHeaders().getContentTypeHeader().mimeTypePart() != null
&& !response.getHeaders().getContentTypeHeader().mimeTypePart().contains("application/zip")) {
String origBody = response.getBodyAsString();
Expand Down

0 comments on commit 29d5461

Please sign in to comment.