Skip to content

tree: fix tuple encoding performance regression#98717

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
mgartner:speed-up-tuple-encoding
Mar 16, 2023
Merged

tree: fix tuple encoding performance regression#98717
craig[bot] merged 1 commit intocockroachdb:masterfrom
mgartner:speed-up-tuple-encoding

Conversation

@mgartner
Copy link
Copy Markdown
Contributor

@mgartner mgartner commented Mar 15, 2023

This commit fixes a performance regression in pgwire encoding of tuples
introduced in #95009.

Informs #98306

Epic: None

Release note: None

@mgartner
tree: fix tuple encoding performance regression
1382571 
This commit fixes a performance regression in pgwire encoding of tuples
introduced in cockroachdb#95009.

Informs cockroachdb#98306

Epic: None

Release note: None
@mgartner mgartner requested review from a team, rafiss and yuzefovich March 15, 2023 21:48
@blathers-crl

This comment was marked as spam.

Sign in to view
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Mar 15, 2023

This change is Reviewable

@mgartner
Copy link
Copy Markdown
Contributor Author

mgartner commented Mar 15, 2023

Some benchmark results:

                                                         │ encoding.master │            encoding.new            │
                                                         │     sec/op      │   sec/op     vs base               │
Encodings/'hello'::char(8)_COLLATE_"en_US"/text-10             91.41n ± 1%   91.68n ± 4%        ~ (p=0.290 n=6)
Encodings/'hello'::char(8)_COLLATE_"en_US"/binary-10           85.76n ± 1%   85.94n ± 2%        ~ (p=0.240 n=6)
Encodings/'hello123'::char(8)_COLLATE_"en_US"/text-10          20.25n ± 0%   20.13n ± 0%   -0.59% (p=0.002 n=6)
Encodings/'hello123'::char(8)_COLLATE_"en_US"/binary-10        15.14n ± 0%   15.05n ± 0%   -0.59% (p=0.004 n=6)
Encodings/(1::char(2)_COLLATE_"en_US",null)/text-10            192.8n ± 1%   117.5n ± 2%  -39.10% (p=0.002 n=6)
Encodings/(1::char(2)_COLLATE_"en_US",null)/binary-10          177.9n ± 2%   176.2n ± 3%        ~ (p=0.240 n=6)
Encodings/(1::char(1)_COLLATE_"en_US",null)/text-10           114.15n ± 1%   48.04n ± 3%  -57.92% (p=0.002 n=6)
Encodings/(1::char(1)_COLLATE_"en_US",null)/binary-10          108.5n ± 1%   109.6n ± 2%   +0.97% (p=0.037 n=6)
Encodings/(1::varchar(4)_COLLATE_"en_US",null)/text-10        112.70n ± 1%   47.85n ± 1%  -57.54% (p=0.002 n=6)
Encodings/(1::varchar(4)_COLLATE_"en_US",null)/binary-10       107.8n ± 2%   108.7n ± 1%   +0.79% (p=0.050 n=6)
Encodings/(1::text_COLLATE_"en_US",null)/text-10              113.10n ± 1%   47.89n ± 0%  -57.65% (p=0.002 n=6)
Encodings/(1::text_COLLATE_"en_US",null)/binary-10             107.5n ± 1%   109.8n ± 1%   +2.14% (p=0.002 n=6)
geomean                                                        85.42n        66.22n       -22.48%

                                                         │ encoding.master │              encoding.new              │
                                                         │      B/op       │    B/op     vs base                    │
Encodings/'hello'::char(8)_COLLATE_"en_US"/text-10            24.00 ± 0%     24.00 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/'hello'::char(8)_COLLATE_"en_US"/binary-10          24.00 ± 0%     24.00 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/'hello123'::char(8)_COLLATE_"en_US"/text-10         0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/'hello123'::char(8)_COLLATE_"en_US"/binary-10       0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/(1::char(2)_COLLATE_"en_US",null)/text-10          226.00 ± 0%     18.00 ± 0%   -92.04% (p=0.002 n=6)
Encodings/(1::char(2)_COLLATE_"en_US",null)/binary-10         226.0 ± 0%     226.0 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/(1::char(1)_COLLATE_"en_US",null)/text-10           208.0 ± 0%       0.0 ± 0%  -100.00% (p=0.002 n=6)
Encodings/(1::char(1)_COLLATE_"en_US",null)/binary-10         208.0 ± 0%     208.0 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/(1::varchar(4)_COLLATE_"en_US",null)/text-10        208.0 ± 0%       0.0 ± 0%  -100.00% (p=0.002 n=6)
Encodings/(1::varchar(4)_COLLATE_"en_US",null)/binary-10      208.0 ± 0%     208.0 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/(1::text_COLLATE_"en_US",null)/text-10              208.0 ± 0%       0.0 ± 0%  -100.00% (p=0.002 n=6)
Encodings/(1::text_COLLATE_"en_US",null)/binary-10            208.0 ± 0%     208.0 ± 0%         ~ (p=1.000 n=6) ¹
geomean                                                                  ²               ?                      ² ³
¹ all samples are equal
² summaries must be >0 to compute geomean
³ ratios must be >0 to compute geomean

                                                         │ encoding.master │              encoding.new              │
                                                         │    allocs/op    │ allocs/op   vs base                    │
Encodings/'hello'::char(8)_COLLATE_"en_US"/text-10            2.000 ± 0%     2.000 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/'hello'::char(8)_COLLATE_"en_US"/binary-10          2.000 ± 0%     2.000 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/'hello123'::char(8)_COLLATE_"en_US"/text-10         0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/'hello123'::char(8)_COLLATE_"en_US"/binary-10       0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/(1::char(2)_COLLATE_"en_US",null)/text-10           4.000 ± 0%     2.000 ± 0%   -50.00% (p=0.002 n=6)
Encodings/(1::char(2)_COLLATE_"en_US",null)/binary-10         4.000 ± 0%     4.000 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/(1::char(1)_COLLATE_"en_US",null)/text-10           2.000 ± 0%     0.000 ± 0%  -100.00% (p=0.002 n=6)
Encodings/(1::char(1)_COLLATE_"en_US",null)/binary-10         2.000 ± 0%     2.000 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/(1::varchar(4)_COLLATE_"en_US",null)/text-10        2.000 ± 0%     0.000 ± 0%  -100.00% (p=0.002 n=6)
Encodings/(1::varchar(4)_COLLATE_"en_US",null)/binary-10      2.000 ± 0%     2.000 ± 0%         ~ (p=1.000 n=6) ¹
Encodings/(1::text_COLLATE_"en_US",null)/text-10              2.000 ± 0%     0.000 ± 0%  -100.00% (p=0.002 n=6)
Encodings/(1::text_COLLATE_"en_US",null)/binary-10            2.000 ± 0%     2.000 ± 0%         ~ (p=1.000 n=6) ¹
geomean                                                                  ²               ?                      ² ³
¹ all samples are equal
² summaries must be >0 to compute geomean
³ ratios must be >0 to compute geomean

yuzefovich
yuzefovich approved these changes Mar 15, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rafiss)

@mgartner mgartner mentioned this pull request Mar 15, 2023
Closed
18 tasks
@mgartner
Copy link
Copy Markdown
Contributor Author

mgartner commented Mar 16, 2023

TFTR!

bors r+

@rafiss
Copy link
Copy Markdown
Collaborator

rafiss commented Mar 16, 2023

I added backport labels since #95009 was also backported.

@mgartner
Copy link
Copy Markdown
Contributor Author

mgartner commented Mar 16, 2023

Thanks @rafiss!

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 16, 2023

Build succeeded:

@craig craig bot merged commit 683545c into cockroachdb:master Mar 16, 2023
This was referenced Mar 16, 2023
Merged
Merged
@mgartner mgartner deleted the speed-up-tuple-encoding branch March 17, 2023 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rafiss rafiss rafiss approved these changes

@yuzefovich yuzefovich yuzefovich approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mgartner @cockroach-teamcity @rafiss @yuzefovich