Optimize equality comparison for small str with fixed size by solotzg · Pull Request #5569 · pingcap/tiflash

solotzg · 2022-08-09T06:52:50Z

What problem does this PR solve?

Issue Number: ref #5294

What is changed and how it works?

optimize equality comparison for small str with fixed size ( size <= 16) like select ... from ... where xxx = 'xxx' ...
add function LoopOneColumnCmpEqFixedStr<> to for such equality comparison
add function memcmp_eq_fixed_size<>.

FLATTEN_INLINE_PURE
inline bool memcmp_eq3(const char * a, const char * b)
{
    return memcmp_eq2(a, b) & memcmp_eq2(a + 1, b + 1);
}

If use && and cpu failed to predict cmp -> jne, the pipeline will be broken.

    movzx   eax, word ptr [rdi]
    cmp     ax, word ptr [rsi]
    jne     ...
    movzx   eax, word ptr [rdi + 1]
    cmp     ax, word ptr [rsi + 1]
    sete    al
    ret

Use & to reduce unnecessary branch. Instructions like (1) and (2) are independent(same for (3) and (4)), it's friendly for parallelism.

    movzx   eax, word ptr [rdi]         // (1)
    movzx   ecx, word ptr [rdi + 1]     // (2)
    xor     ax, word ptr [rsi]          // (3)
    xor     cx, word ptr [rsi + 1]      // (4)
    or      cx, ax
    sete    al
    ret

Benchmark

ENV

tpch-100
1 tiflash
limit cpu up to 200%
original commit: 30fc64c

SQL

select count(1) from lineitem where L_SHIPMODE = 'zzzz';

Time(s)	Original	Optimized		Improvement
	9.15	7.52
	9.33	7.62
	9.12	7.58
	9.23	7.57
	9.14	7.65		AVG(Original) / AVG(Optimized) - 1.0
AVG	9.194	7.588	Optimized : Original	21.16%

SQL

ref tpch-q10

select count(1) from lineitem where L_RETURNFLAG = 'R';

MySQL [tpch100]> select count(1) from lineitem where L_RETURNFLAG = 'R';
+-----------+
| count(1)  |
+-----------+
| 148067261 |
+-----------+

Time(s)	Original	Optimized		Improvement
	12.85	8.56
	12.87	8.64
	12.86	8.45
	12.75	8.51
	12.76	8.64		AVG(Original) / AVG(Optimized) - 1.0
AVG	12.818	8.56	Optimized : Original	49.74%

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

None

ti-chi-bot · 2022-08-09T06:52:51Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

windtalker
zanmato1984

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

solotzg · 2022-08-09T07:56:29Z

/run-all-tests

sre-bot · 2022-08-09T08:34:55Z

Coverage for changed files

Filename                                                Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dbms/src/Columns/ColumnString.cpp                           172                88    48.84%          26                14    46.15%         402               224    44.28%         118                69    41.53%
dbms/src/Functions/CollationOperatorOptimized.h             193               170    11.92%          10                 4    60.00%         232               155    33.19%         104                86    17.31%
dbms/src/Functions/CollationStringSearchOptimized.h         141                 3    97.87%          20                 0   100.00%         302                 5    98.34%          84                 2    97.62%
dbms/src/Functions/FunctionsComparison.h                    604               307    49.17%          63                29    53.97%         949               512    46.05%         476               286    39.92%
dbms/src/Storages/Transaction/Collator.cpp                  223                41    81.61%          39                 5    87.18%         436                83    80.96%         156                60    61.54%
dbms/src/Storages/Transaction/Collator.h                      5                 1    80.00%           5                 1    80.00%           5                 1    80.00%           0                 0         -
libs/libcommon/include/common/fixed_mem_eq.h                 37                37     0.00%           6                 6     0.00%          55                55     0.00%           0                 0         -
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                      1375               647    52.95%         169                59    65.09%        2381              1035    56.53%         938               503    46.38%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18961      9450             50.16%    214500  95526        55.47%

full coverage report (for internal network access only)

solotzg · 2022-08-10T03:24:12Z

/run-all-tests

solotzg · 2022-08-10T03:41:32Z

/run-unit-test

sre-bot · 2022-08-10T04:17:58Z

Coverage for changed files

Filename                                                Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dbms/src/Columns/ColumnString.cpp                           172                87    49.42%          26                14    46.15%         402               222    44.78%         118                67    43.22%
dbms/src/Functions/CollationOperatorOptimized.h             193               170    11.92%          10                 4    60.00%         232               155    33.19%         104                86    17.31%
dbms/src/Functions/CollationStringSearchOptimized.h         141                 3    97.87%          20                 0   100.00%         302                 5    98.34%          84                 2    97.62%
dbms/src/Functions/FunctionsComparison.h                    604               307    49.17%          63                29    53.97%         949               512    46.05%         476               288    39.50%
dbms/src/Storages/Transaction/Collator.cpp                  223                41    81.61%          39                 5    87.18%         436                83    80.96%         156                60    61.54%
dbms/src/Storages/Transaction/Collator.h                      5                 1    80.00%           5                 1    80.00%           5                 1    80.00%           0                 0         -
dbms/src/Storages/Transaction/CollatorUtils.h                26                 4    84.62%           9                 1    88.89%          42                 5    88.10%          10                 1    90.00%
libs/libcommon/include/common/defines.h                       2                 0   100.00%           2                 0   100.00%           8                 0   100.00%           0                 0         -
libs/libcommon/include/common/fixed_mem_eq.h                 39                39     0.00%           7                 7     0.00%          55                55     0.00%           0                 0         -
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                      1405               652    53.59%         181                61    66.30%        2431              1038    57.30%         948               504    46.84%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18962      9454             50.14%    214500  95550        55.45%

full coverage report (for internal network access only)

solotzg · 2022-08-12T01:03:18Z

/run-all-tests

sre-bot · 2022-08-12T01:35:00Z

Coverage for changed files

Filename                                            Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dbms/src/Functions/CollationOperatorOptimized.h         193               164    15.03%          10                 3    70.00%         232                99    57.33%         104                85    18.27%
dbms/src/Storages/Transaction/CollatorUtils.h            26                 4    84.62%           9                 1    88.89%          42                 5    88.10%          10                 1    90.00%
libs/libcommon/include/common/defines.h                   2                 0   100.00%           2                 0   100.00%           8                 0   100.00%           0                 0         -
libs/libcommon/include/common/fixed_mem_eq.h             39                39     0.00%           7                 7     0.00%          55                55     0.00%           0                 0         -
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                   260               207    20.38%          28                11    60.71%         337               159    52.82%         114                86    24.56%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
19264      9251             51.98%    219075  94150        57.02%

full coverage report (for internal network access only)

libs/libcommon/include/common/fixed_mem_eq.h

windtalker

LGTM

libs/libcommon/include/common/fixed_mem_eq.h

zanmato1984

Otherwise LGTM.

…ixed-cmp-coonst

solotzg · 2022-08-16T03:13:34Z

/merge

ti-chi-bot · 2022-08-16T03:13:35Z

@solotzg: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2022-08-16T03:13:36Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: bb8dd6f

solotzg · 2022-08-16T03:31:58Z

/run-integration-test

solotzg · 2022-08-16T03:32:56Z

/run-unit-test

sre-bot · 2022-08-16T04:23:22Z

Coverage for changed files

Filename                                            Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dbms/src/Functions/CollationOperatorOptimized.h         193               164    15.03%          10                 3    70.00%         232                99    57.33%         104                85    18.27%
dbms/src/Storages/Transaction/CollatorUtils.h            26                 4    84.62%           9                 1    88.89%          42                 5    88.10%          10                 1    90.00%
libs/libcommon/include/common/defines.h                   2                 0   100.00%           2                 0   100.00%           8                 0   100.00%           0                 0         -
libs/libcommon/include/common/fixed_mem_eq.h             29                29     0.00%           7                 7     0.00%          60                60     0.00%           0                 0         -
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                   250               197    21.20%          28                11    60.71%         342               164    52.05%         114                86    24.56%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
19266      9250             51.99%    219210  94111        57.07%

full coverage report (for internal network access only)

…ingcap#5569)" This reverts commit 4ab1564.

solotzg added 7 commits August 3, 2022 14:48

optimize rest bin collator

0baa624

fix

3cba3dd

1

d13b7a1

update

67b4764

Refine enum number

ef6b8f9

typo

8e8d659

more comment

89962b8

solotzg added type/enhancement The issue or PR belongs to an enhancement. type/performance labels Aug 9, 2022

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 9, 2022

solotzg mentioned this pull request Aug 9, 2022

Improve the performance of new collation releated function/executor #5294

Closed

9 tasks

Optimize equality comparison for small str with fixed size

f13830c

solotzg force-pushed the opt-fixed-cmp-coonst branch from 67aa355 to f13830c Compare August 10, 2022 02:26

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 11, 2022

solotzg requested review from windtalker and zanmato1984 August 11, 2022 13:30

solotzg force-pushed the opt-fixed-cmp-coonst branch from 2a915ed to f13830c Compare August 11, 2022 14:44

ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 11, 2022

Merge branch 'master' into opt-fixed-cmp-coonst

0fd2baa

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 11, 2022

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 11, 2022

windtalker reviewed Aug 15, 2022

View reviewed changes

libs/libcommon/include/common/fixed_mem_eq.h Show resolved Hide resolved

solotzg requested a review from windtalker August 15, 2022 07:15

windtalker approved these changes Aug 15, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 15, 2022

zanmato1984 reviewed Aug 15, 2022

View reviewed changes

libs/libcommon/include/common/fixed_mem_eq.h Outdated Show resolved Hide resolved

zanmato1984 approved these changes Aug 15, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 15, 2022

solotzg added 3 commits August 16, 2022 10:51

add comments

c04bc98

Merge remote-tracking branch 'origin/opt-fixed-cmp-coonst' into opt-f…

135df6d

…ixed-cmp-coonst

Merge remote-tracking branch 'pingcap/master' into opt-fixed-cmp-coonst

bb8dd6f

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 16, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 16, 2022

ti-chi-bot merged commit 4ab1564 into pingcap:master Aug 16, 2022

solotzg deleted the opt-fixed-cmp-coonst branch August 16, 2022 04:36

solotzg added a commit to solotzg/tiflash that referenced this pull request Sep 19, 2022

Revert "Optimize equality comparison for small str with fixed size (p…

bb6d412

…ingcap#5569)" This reverts commit 4ab1564.

Conversation

solotzg commented Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Benchmark

Check List

Release note

Uh oh!

ti-chi-bot commented Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

solotzg commented Aug 9, 2022

Uh oh!

sre-bot commented Aug 9, 2022

Coverage for changed files

Coverage summary

Uh oh!

solotzg commented Aug 10, 2022

Uh oh!

solotzg commented Aug 10, 2022

Uh oh!

sre-bot commented Aug 10, 2022

Coverage for changed files

Coverage summary

Uh oh!

solotzg commented Aug 12, 2022

Uh oh!

sre-bot commented Aug 12, 2022

Coverage for changed files

Coverage summary

Uh oh!

Uh oh!

windtalker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zanmato1984 left a comment

Choose a reason for hiding this comment

Uh oh!

solotzg commented Aug 16, 2022

Uh oh!

ti-chi-bot commented Aug 16, 2022

Uh oh!

ti-chi-bot commented Aug 16, 2022

Uh oh!

solotzg commented Aug 16, 2022

Uh oh!

solotzg commented Aug 16, 2022

Uh oh!

sre-bot commented Aug 16, 2022

Coverage for changed files

Coverage summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

solotzg commented Aug 9, 2022 •

edited

Loading

ti-chi-bot commented Aug 9, 2022 •

edited

Loading