Skip to content

Preallocate WKB output buffer in AsBinary using exact byte count - #719

Merged
peterstace merged 2 commits into
peterstace:masterfrom
jay23606:pr-go/368-preallocate-byte-slices-in-asbinary-meth
Nov 13, 2026
Merged

peterstace merged 2 commits into
peterstace:masterfrom
jay23606:pr-go/368-preallocate-byte-slices-in-asbinary-meth

Conversation

@jay23606

Copy link
Copy Markdown
Contributor

Bug

Geometry.AsBinary and the per-type AsBinary methods called AppendWKB(nil). A nil slice has zero capacity, so the output buffer was regrown several times while the WKB was written.

Change

  • Each geometry type gets a bytesInWKB() method that computes the exact WKB size. AsBinary allocates make([]byte, 0, g.bytesInWKB()) and appends into it, so the buffer is allocated once.
  • The regression test checks that cap equals len for each geometry type and that AsBinary makes exactly one allocation. A second test checks that bytesInWKB matches len(AsBinary()).
  • BenchmarkMarshalWKB now calls b.ReportAllocs(), so the allocation count is reported alongside timing.

Testing

  • go test ./geom/ -run TestBytesInWKBMatchesAsBinary -count=1 passes on the fix. On the base commit it fails (go -C .pr-go/base test ./geom/ -run TestBytesInWKBMatchesAsBinary -count=1).
  • go test ./geom/ -run TestBytesInWKB -count=1 -v passes on the fix, covering both the length check and the allocation check.
  • go vet ./geom/ is clean.
  • go test ./geom/ -run ^$ -bench BenchmarkMarshalWKB/polygon -benchtime=200x (one run each, so the timings are noisy): on the fix, the n=10000 polygon is 34,124 ns/op with 1 allocs/op. On base, it is 85,596 ns/op.
  • go test ./geom/ -count=1 has one failure, TestGeoJSONSyntaxError.

Regression test: fails on base (go -C .pr-go/base test ./geom/ -run TestBytesInWKBMatchesAsBinary -count=1), passes on fix (go test ./geom/ -run TestBytesInWKBMatchesAsBinary -count=1)

Fixes #368

@peterstace peterstace left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I've got some requests for change.

Comment thread geom/type_bytes_in_wkb_internal_test.go Resolved
Comment thread geom/type_multi_point.go Resolved
@peterstace
peterstace merged commit 685e35d into peterstace:master Nov 13, 2026
1 check passed
@jay23606
jay23606 deleted the pr-go/368-preallocate-byte-slices-in-asbinary-meth branch November 13, 2026 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preallocate byte slices in AsBinary methods

2 participants