slachiewicz opened a new pull request, #3788:
URL: https://github.com/apache/thrift/pull/3788
Client: go
### Summary
Adds an opt-in `struct_key_entries` option to the Go generator:
```bash
thrift --gen go:struct_key_entries file.thrift
```
When enabled, maps keyed by a `struct`, `union`, or `exception` generate as
`[]thrift.MapEntry[*K, V]`, reusing the entry slice representation introduced
in THRIFT-2063 for container keys, and `Equals` compares key contents.
---
### Issues with Existing Go Representation (`map[*K]V`)
By default, Thrift maps keyed by a struct generate in Go as `map[*K]V`.
Because Go maps require hashable keys and structs generated by Thrift contain
pointers/slices, the generator keys the map by pointer (`*K`):
1. **Pointer Identity vs Content Equality**: Decoded keys are freshly
allocated heap objects. Two entries with identical struct contents are distinct
map keys and never collide.
2. **Broken Content Lookups**: Looking up by struct value (`m[&Key{ID: 1}]`)
always fails because the new pointer address never matches the decoded pointer
address stored in the map.
3. **Broken Struct Equality (`Equals`)**: Generated `Equals` compares map
keys by pointer identity. Two separately deserialized messages with identical
field contents compare `false`.
4. **Duplicate Wire Keys**: Because pointer keys are always distinct in
memory, serialization silently writes duplicate struct keys to the wire without
detection.
5. **Struct Typedefs**: A typedef of a struct generates as `type KeyAlias
*Key`. In Go, defined pointer types have empty method sets and cannot be
serialized or compared directly unless unwrapped to the underlying struct
pointer.
---
### Performance & Memory Impact (New vs Old)
#### Memory Layout & Allocations
- **Reduced Memory Overhead**: A Go `map` requires an `hmap` header (~48
bytes) plus hash buckets (`bmap`) with 8-entry chunks, tophash, overflow
pointers, and bucket slack (typically 20–40% unused load factor space). In
contrast, `[]MapEntry[*K, V]` is a flat 24-byte slice header pointing to a
single contiguous array, reducing container memory footprint by ~30–50%.
- **L1/L2 Cache Locality**: Slices provide linear memory layout and
prefetching, avoiding hash table pointer chasing.
- **GC Overhead**: Lowers garbage collector scanning pressure by replacing
scattered bucket allocations with a single contiguous buffer.
#### Deserialization (Read)
- **Faster**: Deserialization reads count $N$ from the wire and allocates
the exact capacity in a single heap allocation (`make([]MapEntry, 0, N)`),
appending elements directly without Go runtime map hashing, bucket resolution,
or map growth.
#### Serialization (Write)
- **Wire Correctness Guarantee**: Writing `[]MapEntry` executes a pairwise
duplicate key check (`for i ... for j := i+1 ... if
entries[i].Key.Equals(...)`) and returns `thrift.INVALID_DATA` on duplicates
(matching how Thrift Go sets and container keys behave).
- **$O(N^2)$ Pairwise Check**:
- For typical Thrift maps ($N \le 20$), overhead is negligible.
- For very large maps ($N \ge 1,000$), write serialization is slower due
to pairwise comparisons compared to unchecked map serialization.
#### Equality (`Equals`)
- **Semantic Correctness**: Compares key and value contents instead of
pointer identity.
- **Order Sensitivity**: Position-by-position comparison makes equality
order-sensitive ($O(N)$), whereas map iteration order was undefined.
---
### Compatibility
- **Fully Opt-In**: The default behavior remains `map[*K]V` unless
`go:struct_key_entries` is explicitly specified.
- Primitive keys (`map[string]int`, etc.) and container keys (`[]MapEntry`
from THRIFT-2063) remain unaffected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
