slachiewicz commented on PR #3788: URL: https://github.com/apache/thrift/pull/3788#issuecomment-5551256081
Force-pushed after your approval, so flagging what changed. The write-side uniqueness check is no longer always quadratic. When every field of the key struct is a non-pointer scalar, the struct value is a valid Go map key and `==` on it agrees with the generated `Equals`, so the keys go into a set. Anything else keeps the pairwise scan: optional fields, nested structs, and `binary`/`list`/`set`/`map` fields do not compare by content under `==`, and unions have only optional fields. Container keys never qualify, so THRIFT-2063 is untouched. Over 5000 struct keys, `go test -bench 5000 -count 5` medians: 518 µs, against 437 µs for the same data as `map[*K]V`. The pairwise scan took 75.9 ms. Also in this push: two nil keys now count as duplicates, matching what `Equals` reports for them; `StructKeyTest.thrift` gains list and set values; and there are new tests for an all-empty round trip, a union key with no field set, and nil keys. The README section is rewritten to match. One thing the README now states explicitly: `Equals` on an entry slice is positional, so it is set equality only when both sides share an order. A round trip is stable, but two independent producers of the same logical map compare unequal. That is still strictly better than `map[*K]V`, whose `Equals` is false even between a value and its own decode. Making it order-insensitive for the qualifying key shape is a natural follow-up, though it would then disagree with the container-key `Equals` that THRIFT-2063 already merged. Happy to file that separately if you want it. *This comment was created with AI assistance.* -- 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]
