Conversation
bind() allocated every []*Struct element through makeStructPtr(), which
walked each field of the struct type and parsed its boil tag looking for
the ",bind" pointer fields that need allocating. The struct type is
fixed for the lifetime of a bind, so this recomputed the same answer on
every row: a 1M-row query over a 20-column model ran 20M redundant tag
parses.
The per-type mappingCache already caches column mappings, so the field
scan moves there as bindPtrFields, built once in newMappingCache and
consumed by the new newStructPtr method. It is fixed at construction
time, so it is read without holding the cache mutex.
Binding 1000 rows, benchstat over 6 runs:
BindSmallPtrSlice 312.3µ -> 176.5µ -43.50%
BindWidePtrSlice 1344.3µ -> 687.8µ -48.83%
The []Struct paths and the allocation counts are unchanged, since the
work removed was pure CPU.
Adds TestBind_InnerJoinPtrFields, which covers the allocation of ",bind"
pointer fields. No existing test bound to a struct with one, despite it
being the documented example on Bind.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
|
It is a minor performance improvement but I believe the improvement is monotonic. I also wrote a small benchmark test - which I could add if folks thinks that is better. |
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.
bind() allocated every []*Struct element through makeStructPtr(), which walked each field of the struct type and parsed its boil tag looking for the "bind" pointer fields that need allocating. The struct type is fixed for the lifetime of a bind, so this recomputed the same answer on every row: a 1M-row query over a 20-column model ran 20M redundant tag parses.
The per-type mappingCache already caches column mappings, so the field scan moves there as bindPtrFields, built once in newMappingCache and consumed by the new newStructPtr method. It is fixed at construction time, so it is read without holding the cache mutex.
Binding 1000 rows, benchstat over 6 runs:
The []Struct paths and the allocation counts are unchanged, since the work removed was pure CPU.
Adds TestBind_InnerJoinPtrFields, which covers the allocation of ",bind" pointer fields. No existing test bound to a struct with one, despite it being the documented example on Bind.