copy: optimize date parsing memory usage#92252
copy: optimize date parsing memory usage#92252craig[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
Simple benchmark for ParseDate to highlight forthcoming optimizations. Release note: None
5e996cd to
f12108b
Compare
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 5 of 5 files at r2, 4 of 4 files at r3, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @cucaroach and @otan)
pkg/sql/colexec/execgen/cmd/execgen/cast_gen_util.go line 398 at r3 (raw file):
_now := %[3]s.GetRelativeParseTime() _dateStyle := %[3]s.GetDateStyle() _d, _, err := pgdate.ParseDate(_now, _dateStyle, string(%[2]s), &evalCtx.ParseHelper)
nit: replace evalCtx with %[3]s as we do for _now and _dateStyle. The rationale is trying to keep the templating more flexible (although in this case the flexibility is probably not worth it since I don't foresee us generating multiple files based on cast_tmpl, still it's better for consistency sake).
pkg/sql/sem/tree/datum.go line 2008 at r3 (raw file):
// GetDateStyle returns the date style in the session. GetDateStyle() pgdate.DateStyle // GetParseHelper returns a helper to optmize date parsing
nit: missing period.
pkg/util/timeutil/pgdate/field_extract.go line 78 at r2 (raw file):
// This indicates that the value in the year field was only // two digits and should be adjusted to make it recent. tweakYear bool
nit: I'm assuming that the fields are re-arranged to get the object size under some threshold. Consider adding an assertion in init() method that verifies that the size doesn't exceed that threshold.
f12108b to
a1a6352
Compare
cucaroach
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @otan and @yuzefovich)
pkg/util/timeutil/pgdate/field_extract.go line 78 at r2 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: I'm assuming that the fields are re-arranged to get the object size under some threshold. Consider adding an assertion in
init()method that verifies that the size doesn't exceed that threshold.
Seems like overkill, I just wanted to save 16 bytes by collapsing bools, it probably didn't move the needle, just seemed like a good idea.
|
TFTRs! |
|
Build failed: |
a1a6352 to
c2b613e
Compare
Add a "helper" to cache memory allocations to make repeated/bulk date parsing more efficient. ``` name old time/op new time/op delta ParseDate-10 1.76µs ±12% 1.49µs ± 6% -15.08% (p=0.000 n=9+10) name old alloc/op new alloc/op delta ParseDate-10 1.66kB ± 0% 0.00kB -100.00% (p=0.000 n=10+10) name old allocs/op new allocs/op delta ParseDate-10 8.00 ± 0% 0.00 -100.00% (p=0.000 n=10+10) ``` Fixes: cockroachdb#91834 Release note: None
c2b613e to
048f833
Compare
|
bors r+ |
|
Build failed (retrying...): |
|
Build failed: |
|
bors r+ |
|
Build failed (retrying...): |
|
Build succeeded: |
Fixes: #91834
I'll squash the last two commits but thought for review purposes they'd be better separate.
Epic: CRDB-18892