Repository navigation
geom: build nested GeometryCollections in linear time - #717
Merged
Merged
Conversation
NewGeometryCollection forced every member to the collection's coordinates type, and GeometryCollection.ForceCoordinatesType rebuilt every member recursively even when the type was unchanged. Building a collection N levels deep, as the parsers do one level at a time, therefore copied the innermost members N times. Only force members whose coordinates type differs, and return the collection as is from ForceCoordinatesType when nothing changes. Add BenchmarkUnmarshalWKB/nested_collection.
Owner
|
Thanks for the change! |
peterstace
approved these changes
Nov 11, 2026
1 of 13 tasks
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.
Description
Fixes #716 "Parsing nested GeometryCollections is quadratic in the nesting
depth".
NewGeometryCollectionforced every member to the collection's coordinatestype, and
GeometryCollection.ForceCoordinatesTyperebuilt all of its membersrecursively even when the coordinates type was already the requested one. The
parsers build nested collections bottom up, one
NewGeometryCollectioncallper level, so a collection nested N levels deep had its innermost members
copied N times.
This change:
ForceCoordinatesTypewhen it already hasthe requested coordinates type.
Results are unchanged (apart from the
reflect.DeepEqualdetail for emptycollections noted in #716). Members that need a different coordinates type are
still forced, including inside nested collections. Parsing non-collection
geometries is unaffected, and flat collections get slightly cheaper since
members are no longer re-wrapped.
I used Claude Code to prepare this change.
Check List
Have you:
Added unit tests? Yes, in the new
geom/type_geometry_collection_test.go:TestNewGeometryCollectionDoesNotCopyNestedMembers: wrapping a 100 deepcollection allocates no more than wrapping a 1 deep one (204 vs 6
allocations before this change).
TestGeometryCollectionForceCoordinatesTypeUnchanged: forcing to thecurrent coordinates type allocates nothing and returns an equal value (201
allocations before this change).
TestNewGeometryCollectionForcesNestedMembers: a nestedPOINT Zisstill forced to XY when combined with an XY point (passes before and
after).
Add cmprefimpl tests? Not appropriate, results are unchanged.
Updated release notes? Yes, an entry in the Unreleased section of
CHANGELOG.md.Updated the README.md? Not applicable, no new functionality.
Related Issue
Benchmark Results
BenchmarkUnmarshalWKB/nested_collection, which parses a 1,000 pointMultiPointwrapped in 1, 10 and 100 levels ofGeometryCollection. Belowis
benchstatover 8 interleaved runs ofBenchmarkUnmarshalWKBandBenchmarkWKTParsing(run withgo test -benchrather thanrun_benchmarks.sh).Click to expand