Repository navigation
Require Go 1.19 and use its encoding/binary append functions - #715
Merged
Merged
Conversation
Raise the go directive in go.mod and the golang image for the unit, pgscan, and cmppg CI jobs from 1.18 to 1.19. The geos and proj jobs already build with Go 1.21, and golangci-lint v1.61.0 in the lint job supports 1.19.
Go 1.19 added AppendUint32 and AppendUint64 to binary.LittleEndian and binary.BigEndian through the binary.AppendByteOrder interface, along with binary.AppendVarint and binary.AppendUvarint. The WKB and TWKB encoders now append to the output buffer directly instead of encoding each value into a temporary array first. The WKB encoder passed its temporary array to a method on the binary.ByteOrder interface, which made the array escape to the heap. Each header field and each Point or MultiPoint coordinate cost an allocation. The TWKB bbox buffer is created with room for the largest possible header, so appending its varints one at a time never grows it. Add a multipoint case to BenchmarkMarshalWKB and a new BenchmarkMarshalTWKB. The existing polygon case writes coordinates with one bulk copy, so it doesn't exercise the per-value code.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are blocking approval.
No open issues 🎉
What changed in this PR
Raises the module’s minimum Go version to 1.19 and uses its encoding/binary append APIs to improve WKB and TWKB encoding performance without changing encoded output.
Changes:
- Updates the module requirement and three CI jobs to Go 1.19.
- Replaces temporary-buffer encoding with direct append calls.
- Adds targeted benchmarks and documents compatibility and performance changes.
| File | Description |
|---|---|
go.mod |
Raises the minimum Go version to 1.19. |
geom/wkb_marshal.go |
Uses append APIs for WKB header fields and coordinates. |
geom/twkb_write.go |
Uses append APIs for TWKB varints and headers. |
geom/perf_test.go |
Adds WKB multipoint and TWKB polygon benchmarks. |
CHANGELOG.md |
Records the Go requirement and encoder performance improvements. |
.ci/compose-unit.yaml |
Runs unit tests on Go 1.19. |
.ci/compose-pgscan.yaml |
Runs PostgreSQL scanning tests on Go 1.19. |
.ci/compose-cmppg.yaml |
Runs PostGIS comparison tests on Go 1.19. |
Review effort: Balanced
peterstace
commented
Nov 6, 2026
| // Store bbox min and delta for each dimension. | ||
| var buf [twkbMaxDimensions * 2 * binary.MaxVarintLen64]byte | ||
| n := 0 | ||
| w.twkbBBox = make([]byte, 0, 2*w.dimensions*binary.MaxVarintLen64) |
Owner
Author
There was a problem hiding this comment.
This assigns rather than appends, so it relies on twkbBBox being empty at this point. It always is: writeBBoxHeader only runs from writeGeometry, which runs once per writer, and the sub-writers from copytwkbWriter never write a bbox. The capacity fits the largest possible header, so appending the varints never grows the buffer. Without it, appending to a nil slice cost small geometries an extra allocation.
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
This raises the minimum Go version from 1.18 to 1.19 and changes the WKB and TWKB encoders to use the
encoding/binaryappend functions that 1.19 added.go.modnow declaresgo 1.19, and the unit, pgscan, and cmppg CI jobs run ongolang:1.19. The geos and proj jobs already build withgolang:1.21-alpine, and golangci-lint v1.61.0 in the lint job supports 1.19, so neither changes.Go 1.19 added
AppendUint32andAppendUint64tobinary.LittleEndianandbinary.BigEndianthrough the newbinary.AppendByteOrderinterface, along withbinary.AppendVarintandbinary.AppendUvarint. The encoders used to encode each value into a temporary array and then append the array to the output. They now append to the output directly. In the WKB encoder the temporary array was passed to a method on thebinary.ByteOrderinterface, which made it escape to the heap, so each header field and eachPointorMultiPointcoordinate cost an allocation.nativeOrderchanges type frombinary.ByteOrdertobinary.AppendByteOrderto expose the append methods. The comparisons againstbinary.BigEndianandbinary.LittleEndianinwkb_parser.gobehave the same.Nothing else in the Go 1.19 release notes fits the codebase:
Doc links (
[Name]) are already used throughout the doc comments.sort.Findwould suit the binary search ininternal/jtsport/jts/noding_segment_node_list.go, butinternal/jtsportis out of scope here.The typed atomics,
fmt.Append*, the other new APIs, and theunixbuild constraint don't have anything to replace.The new sort algorithm and the switch jump tables don't need any code changes. The unit tests pass on 1.19, so no test depends on how an unstable sort orders equal elements.
Check List
Have you:
Added unit tests? No. The existing WKB and TWKB tests cover the changed encoders, and the encoded bytes are the same as before. The PR does add benchmarks for the changed code, described under Benchmark Results.
Add cmprefimpl tests? (if appropriate?) Not appropriate, since there is no new behaviour to compare.
make cmppg, which comparesAsBinaryoutput byte for byte against PostGIS, passes on Go 1.19.Updated release notes? (if appropriate?) Yes. The
Unreleasedsection ofCHANGELOG.mdhas an entry for the new minimum Go version, marked as a breaking change, and one for the encoder performance.Updated the README.md (if new functionality is added?) Not appropriate. There's no new functionality, and the README doesn't state a Go version.
Related Issue
Benchmark Results
These come from
go test -benchon linux/arm64, with 10 runs each of the base commit and this branch, interleaved. The base checkout had this PR'sperf_test.gocopied in so that both sides run the same benchmarks.run_benchmarks.shwasn't used, since it runs every benchmark in the module. The existingMarshalWKB/polygoncases write coordinates with one bulk copy, which this PR doesn't change. The newMarshalWKB/multipointandMarshalTWKB/polygoncases cover the per-value code that did change.Click to expand