Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
"mapd generate" checks every scanned way against every 0.25 degree area in the box, so a 20x20 box does 6,400 passes over all of its ways. This builds the list of ways for each area in one pass instead, checking each way only against the areas its bounding box (plus the overlap) can reach. It's the same "Overlapping" test on the same ways in the same order, so the tiles come out byte for byte the same.
Audit report
Why it's slow
The "Finding Bounds" loop runs once per area, and inside it loops over all of "scannedWays". For a 20x20 box that's 6,400 times the number of ways, even though each way only touches the few areas around it. The time scales with ways times areas, so the dense boxes in the planet run (Europe, the eastern US) take the longest.
Change
Before the area loop, each way's bounding box, widened by "|overlap|", is turned into a range of area indexes with one extra area on each side so rounding at the edges can't drop one. Each area in that range gets the same "way.Box.Overlapping(area.OverlapBox(s.Overlap))" check as before, and the way's index goes into that area's list if it passes. Ways are visited in scan order, so each area's list is in the same order as before. The area loop then reads its list instead of scanning everything. "areaIndexRange" uses the same "-90 + i*AREA_BOX_DEGREES" grid as "areaBox", so it doesn't assume anything new about the grid. The per-area lists are one slice per area in the world grid, about 25 MB of slice headers on every run, which is where most of the small memory increase on the dense box comes from.
A closed PR of mine (#113) tried a grouped index and I closed it myself. It added an O(ways x groups) prepass, made one or two area selections slower, and narrowed what "AREA_BOX_DEGREES" could be. This version has none of those. The cost per way is the handful of areas it touches, and it works for any "AREA_BOX_DEGREES". Small selections on a large input can be a little slower, between 0.3 s faster and 0.4 s slower in single runs, since every scanned way is indexed whatever the selection. The planet pipeline always selects the whole 20x20 box it extracted, so it only sees the speedup.
Testing
I ran "filter_planet.sh", "extract_box.sh" and "add_locations.sh" unmodified on the Geofabrik Oregon extract, then ran "mapd generate" from main and from this change on the same "map.osm.pbf" and compared every output file with "diff -r" and per-file sha256. All 12 runs were identical.
Timing above is from three runs of each 20x20 box in "golang:1.26.2-bookworm" on a 4 core VM with other containers running. "go build ./...", "go vet ./..." and "go test ./..." pass.
Not covered
I didn't run a whole-world selection or the full planet. A non-finite "--overlap" (infinity) would give different results, but nobody passes that.