Migrate @turf/polygonize to Map/Set - #3074
Merged
Merged
Conversation
When looking at #3045 it occurred to me that this code probably hasn't been updated since we moved from ES5 to ES8 support in Turf. Specifically: Node was getting a very log string assigned as its id, and then we were using an Object keyed by those ids to do lookups. This changes Node's id to a number (incremented as Graph creates them). Graph keeps both a Set<Node> and an index of Map<longitude, Map<latitude, Node>> used to quickly figure out if a Node has already been created for a given set of coordinates for reuse.
6 tasks
mfedderly
marked this pull request as ready for review
June 22, 2026 00:29
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
1 participant
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.
When looking at #3045 it occurred to me that this code probably hasn't been updated since we moved from ES5 to ES8 support in Turf. Specifically: Node was getting a very log string assigned as its id, and then we were using an
Objectkeyed by those strings to do lookups. These are both suspiciously expensive so I figured I'd take a look.This changes
Node's id to a number (incremented as Graph creates them).Graphkeeps both aSet<Node>and an index ofMap<longitude, Map<latitude, Node>>used to quickly figure out if aNodehas already been created for a given set of coordinates for reuse.Some benchmarks (M4 Macbook Pro, on battery) shows a pretty nice improvement overall:
Before:
complex x 2,520 ops/sec ±0.42% (98 runs sampled)
cutedge x 216,103 ops/sec ±0.22% (95 runs sampled)
dangle x 412,099 ops/sec ±0.35% (95 runs sampled)
kinked-linestring x 315,924 ops/sec ±0.25% (99 runs sampled)
linestrings x 228,949 ops/sec ±0.18% (97 runs sampled)
multi-linestring x 393,533 ops/sec ±0.20% (98 runs sampled)
two-polygons x 224,014 ops/sec ±0.22% (97 runs sampled)
After this PR:
complex x 4,511 ops/sec ±0.34% (98 runs sampled)
cutedge x 266,407 ops/sec ±0.24% (100 runs sampled)
dangle x 530,973 ops/sec ±0.28% (97 runs sampled)
kinked-linestring x 383,615 ops/sec ±0.17% (98 runs sampled)
linestrings x 280,813 ops/sec ±3.76% (93 runs sampled)
multi-linestring x 441,141 ops/sec ±2.94% (90 runs sampled)
two-polygons x 262,369 ops/sec ±0.13% (98 runs sampled)