planetscale / planetscale/vtprotobuf
Better support for maps
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 1.1k
- Forks
- 112
- PR merge metrics
- No merged PRs in 30d
Description
Hello,
While testing a bit this library in our code base, I noted that map support with pool is not perfect.
- map themselves are not pooled
- ResetVT does not return to pool values from a map<key, MessagePooled>
- UnmarshalVT allocates a new message instead of using the pool for map<key, MessagePooled>
Doing these changes manually in the .pb.go reduces a lot allocations and speeds up unmarshalling.
I can provide a sample proto file and modifications made if needed.
Pre changes:
BenchmarkUnmarshalStdProto-12 202058 4971 ns/op 3840 B/op 54 allocs/op
BenchmarkUnmarshalVTProto-12 228591 5238 ns/op 3429 B/op 47 allocs/op
BenchmarkUnmarshalVTProtoWithPool-12 238689 4967 ns/op 2605 B/op 44 allocs/op
Post changes:
BenchmarkUnmarshalStdProto-12 203602 5240 ns/op 3840 B/op 54 allocs/op
BenchmarkUnmarshalVTProto-12 199917 5864 ns/op 3433 B/op 47 allocs/op
BenchmarkUnmarshalVTProtoWithPool-12 601562 2009 ns/op 302 B/op 5 allocs/op
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
The issue does not name files or tests; begin by locating the generator code responsible for map handling in generated .pb.go files and review the existing pool paths. Reproduce the supplied benchmarks with map<key, MessagePooled> fields, then verify that maps and their values are pooled by ResetVT and UnmarshalVT without regressing behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- performance, tooling
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100