-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sql/pgwire: avoid new memory allocation in writeTextDatum(Date) #41403
sql/pgwire: avoid new memory allocation in writeTextDatum(Date) #41403
Conversation
3dfd6cb introduced a performance regression in the speed of encoding Date datums to their text representation. This commit partially avoids the regression. ``` name old time/op new time/op delta WriteTextDate-16 446ns ± 4% 318ns ± 1% -28.75% (p=0.000 n=10+9) ``` Release justification: Avoids performance regression. Release note: None
The overwhelming majority of the remaining time is spent in Unless we want to get very manual and do something like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Also good catch. |
bors r+ |
41399: sql/pgwire: fix BenchmarkWriteBinary{Int,Float} r=nvanbenschoten a=nvanbenschoten Before this fix, the benchmark was always hitting an error because it was forgetting to pass an OID `writeBuffer.writeBinaryDatum`. This commit fixes the benchmark and checks the error to make sure we catch bugs like this in the future. Release justification: Testing only. Release note: None 41400: disable release justification hint and lint. r=RaduBerinde a=RaduBerinde This effectively disables commit feeaf62. The functionality is moved behind flags so it can easily be revived for next release. Release justification: how ironic. Release note: None 41403: sql/pgwire: avoid new memory allocation in writeTextDatum(Date) r=nvanbenschoten a=nvanbenschoten 3dfd6cb introduced a performance regression in the speed of encoding Date datums to their text representation. This commit partially avoids the regression. ``` name old time/op new time/op delta WriteTextDate-16 446ns ± 4% 318ns ± 1% -28.75% (p=0.000 n=10+9) ``` Release justification: Avoids performance regression. Release note: None 41406: Revert "Merge #41307" r=andreimatei a=andreimatei This reverts commit 1c99165, reversing changes made to e6e9161. Somehow #41307 broke some Jepsen tests. Reversing while I investigate. Fixes #41376 Fixes #41364 Fixes #41363 Fixes #41362 Release justification: broke Jepsen Release note: None Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Radu Berinde <[email protected]> Co-authored-by: Andrei Matei <[email protected]>
Build succeeded |
3dfd6cb introduced a performance regression in the speed of encoding
Date datums to their text representation. This commit partially avoids
the regression.
Release justification: Avoids performance regression.
Release note: None