[libc++] Optimize lexicographical_compare #65279

philnik777 · 2023-09-04T18:06:14Z

If the comparison operation is equivalent to < and that is a total order, we know that we can use equality comparison on that type instead to extract some information. Furthermore, if equality comparison on that type is trivial, the user can't observe that we're calling it. So instead of using the user-provided total order, we use std::mismatch, which uses equality comparison (and is vertorized). Additionally, if the type is trivially lexicographically comparable, we can go one step further and use std::memcmp directly instead of calling std::mismatch.

Benchmarks:

-------------------------------------------------------------------------------------
Benchmark                                                         old             new
-------------------------------------------------------------------------------------
bm_lexicographical_compare<unsigned char>/1                   1.17 ns         2.34 ns
bm_lexicographical_compare<unsigned char>/2                   1.64 ns         2.57 ns
bm_lexicographical_compare<unsigned char>/3                   2.23 ns         2.58 ns
bm_lexicographical_compare<unsigned char>/4                   2.82 ns         2.57 ns
bm_lexicographical_compare<unsigned char>/5                   3.34 ns         2.11 ns
bm_lexicographical_compare<unsigned char>/6                   3.94 ns         2.21 ns
bm_lexicographical_compare<unsigned char>/7                   4.56 ns         2.11 ns
bm_lexicographical_compare<unsigned char>/8                   5.25 ns         2.11 ns
bm_lexicographical_compare<unsigned char>/16                  9.88 ns         2.11 ns
bm_lexicographical_compare<unsigned char>/64                  38.9 ns         2.36 ns
bm_lexicographical_compare<unsigned char>/512                  317 ns         6.54 ns
bm_lexicographical_compare<unsigned char>/4096                2517 ns         41.4 ns
bm_lexicographical_compare<unsigned char>/32768              20052 ns          488 ns
bm_lexicographical_compare<unsigned char>/262144            159579 ns         4409 ns
bm_lexicographical_compare<unsigned char>/1048576           640456 ns        20342 ns
bm_lexicographical_compare<signed char>/1                     1.18 ns         2.37 ns
bm_lexicographical_compare<signed char>/2                     1.65 ns         2.60 ns
bm_lexicographical_compare<signed char>/3                     2.23 ns         2.83 ns
bm_lexicographical_compare<signed char>/4                     2.81 ns         3.06 ns
bm_lexicographical_compare<signed char>/5                     3.35 ns         3.30 ns
bm_lexicographical_compare<signed char>/6                     3.90 ns         3.99 ns
bm_lexicographical_compare<signed char>/7                     4.56 ns         3.78 ns
bm_lexicographical_compare<signed char>/8                     5.20 ns         4.02 ns
bm_lexicographical_compare<signed char>/16                    9.80 ns         6.21 ns
bm_lexicographical_compare<signed char>/64                    39.0 ns         3.16 ns
bm_lexicographical_compare<signed char>/512                    318 ns         7.58 ns
bm_lexicographical_compare<signed char>/4096                  2514 ns         47.4 ns
bm_lexicographical_compare<signed char>/32768                20096 ns          504 ns
bm_lexicographical_compare<signed char>/262144              156617 ns         4146 ns
bm_lexicographical_compare<signed char>/1048576             624265 ns        19810 ns
bm_lexicographical_compare<int>/1                             1.15 ns         2.12 ns
bm_lexicographical_compare<int>/2                             1.60 ns         2.36 ns
bm_lexicographical_compare<int>/3                             2.21 ns         2.59 ns
bm_lexicographical_compare<int>/4                             2.74 ns         2.83 ns
bm_lexicographical_compare<int>/5                             3.26 ns         3.06 ns
bm_lexicographical_compare<int>/6                             3.81 ns         4.53 ns
bm_lexicographical_compare<int>/7                             4.41 ns         4.72 ns
bm_lexicographical_compare<int>/8                             5.08 ns         2.36 ns
bm_lexicographical_compare<int>/16                            9.54 ns         3.08 ns
bm_lexicographical_compare<int>/64                            37.8 ns         4.71 ns
bm_lexicographical_compare<int>/512                            309 ns         24.6 ns
bm_lexicographical_compare<int>/4096                          2422 ns          204 ns
bm_lexicographical_compare<int>/32768                        19362 ns         1947 ns
bm_lexicographical_compare<int>/262144                      155727 ns        19793 ns
bm_lexicographical_compare<int>/1048576                     623614 ns        80180 ns
bm_ranges_lexicographical_compare<unsigned char>/1            1.07 ns         2.35 ns
bm_ranges_lexicographical_compare<unsigned char>/2            1.72 ns         2.13 ns
bm_ranges_lexicographical_compare<unsigned char>/3            2.46 ns         2.12 ns
bm_ranges_lexicographical_compare<unsigned char>/4            3.17 ns         2.12 ns
bm_ranges_lexicographical_compare<unsigned char>/5            3.86 ns         2.12 ns
bm_ranges_lexicographical_compare<unsigned char>/6            4.55 ns         2.12 ns
bm_ranges_lexicographical_compare<unsigned char>/7            5.25 ns         2.12 ns
bm_ranges_lexicographical_compare<unsigned char>/8            5.95 ns         2.13 ns
bm_ranges_lexicographical_compare<unsigned char>/16           11.7 ns         2.13 ns
bm_ranges_lexicographical_compare<unsigned char>/64           45.5 ns         2.36 ns
bm_ranges_lexicographical_compare<unsigned char>/512           366 ns         6.35 ns
bm_ranges_lexicographical_compare<unsigned char>/4096         2886 ns         40.9 ns
bm_ranges_lexicographical_compare<unsigned char>/32768       23054 ns          489 ns
bm_ranges_lexicographical_compare<unsigned char>/262144     185302 ns         4339 ns
bm_ranges_lexicographical_compare<unsigned char>/1048576    741576 ns        19430 ns
bm_ranges_lexicographical_compare<signed char>/1              1.10 ns         2.12 ns
bm_ranges_lexicographical_compare<signed char>/2              1.66 ns         2.35 ns
bm_ranges_lexicographical_compare<signed char>/3              2.23 ns         2.58 ns
bm_ranges_lexicographical_compare<signed char>/4              2.82 ns         2.82 ns
bm_ranges_lexicographical_compare<signed char>/5              3.34 ns         3.06 ns
bm_ranges_lexicographical_compare<signed char>/6              3.92 ns         3.99 ns
bm_ranges_lexicographical_compare<signed char>/7              4.64 ns         4.10 ns
bm_ranges_lexicographical_compare<signed char>/8              5.21 ns         4.61 ns
bm_ranges_lexicographical_compare<signed char>/16             9.79 ns         7.42 ns
bm_ranges_lexicographical_compare<signed char>/64             38.9 ns         2.93 ns
bm_ranges_lexicographical_compare<signed char>/512             317 ns         7.31 ns
bm_ranges_lexicographical_compare<signed char>/4096           2500 ns         47.5 ns
bm_ranges_lexicographical_compare<signed char>/32768         19940 ns          496 ns
bm_ranges_lexicographical_compare<signed char>/262144       159166 ns         4393 ns
bm_ranges_lexicographical_compare<signed char>/1048576      638206 ns        19786 ns
bm_ranges_lexicographical_compare<int>/1                      1.10 ns         2.12 ns
bm_ranges_lexicographical_compare<int>/2                      1.64 ns         3.04 ns
bm_ranges_lexicographical_compare<int>/3                      2.23 ns         2.58 ns
bm_ranges_lexicographical_compare<int>/4                      2.81 ns         2.81 ns
bm_ranges_lexicographical_compare<int>/5                      3.35 ns         3.05 ns
bm_ranges_lexicographical_compare<int>/6                      3.94 ns         4.60 ns
bm_ranges_lexicographical_compare<int>/7                      4.60 ns         4.81 ns
bm_ranges_lexicographical_compare<int>/8                      5.19 ns         2.35 ns
bm_ranges_lexicographical_compare<int>/16                     9.85 ns         2.87 ns
bm_ranges_lexicographical_compare<int>/64                     38.9 ns         4.70 ns
bm_ranges_lexicographical_compare<int>/512                     318 ns         24.5 ns
bm_ranges_lexicographical_compare<int>/4096                   2494 ns          202 ns
bm_ranges_lexicographical_compare<int>/32768                 20000 ns         1939 ns
bm_ranges_lexicographical_compare<int>/262144               160433 ns        19730 ns
bm_ranges_lexicographical_compare<int>/1048576              642636 ns        80760 ns

EricWF · 2023-09-05T20:55:25Z

Can you please add benchmarks for types other than int or other small integral types.
Making an algorithm faster for types that fit in registers and are cheap to copy isn't the whole story, and might actually hide the pessimization for other types.

std::string is a good candidate for a test type. With both short and long strings.

EricWF · 2023-09-05T21:51:52Z

Please add a description of how you optimized the code. It's not at all clear after reading the diff.

EricWF · 2023-09-05T23:21:06Z

Please take a look at this godbolt.

It looks like under optimized builds, apart from for unsigned char, there is literally no difference in generated assembly.
However, for non-optimized builds, the new implementation is like 3x the size of the old one.

I don't think that cost/benefit trade off has the correct balance.

Convince me otherwise?

EricWF · 2023-09-05T23:23:49Z

@philnik777 Also, please include descriptions. IT took me an hour to realize this change only affects unsigned char.

philnik777 · 2023-09-06T01:14:16Z

Please take a look at this godbolt.

It looks like under optimized builds, apart from for unsigned char, there is literally no difference in generated assembly. However, for non-optimized builds, the new implementation is like 3x the size of the old one.

I don't think that cost/benefit trade off has the correct balance.

Convince me otherwise?

Could you share the link you were looking at?

@philnik777 Also, please include descriptions. IT took me an hour to realize this change only affects unsigned char.

This is only a draft, so I didn't bother writing a comment. This patch isn't ready for review yet.

EricWF · 2023-09-06T18:59:23Z

@philnik777 Please don't open pull requests for changes that aren't ready to review.

I'll go ahead and close this. Please re-submit when you're ready.

philnik777 · 2023-09-06T23:21:49Z

@philnik777 Please don't open pull requests for changes that aren't ready to review.

I'll go ahead and close this. Please re-submit when you're ready.

This is a draft. Drafts are for patches that are not ready for review.

EricWF · 2023-09-07T16:54:26Z

Drafts seem to prevent merging, not review.

Is there a way to filter pull requests to non-draft libc++ PR's? If not, then I think we need to reconsider the purpose of drafts.

What is the purpose of opening the draft if you didn't want review?

philnik777 · 2023-09-07T17:46:38Z

Drafts seem to prevent merging, not review.

Yes. It also prevents notifying people until it is made a PR.

Is there a way to filter pull requests to non-draft libc++ PR's? If not, then I think we need to reconsider the purpose of drafts.

-is:draft label:libc++

What is the purpose of opening the draft if you didn't want review?

It has multiple benefits:

People see what I'm working on and don't duplicate it
I can work on any comments etc. for the PR
I can easily view the diff and make sure that the patch applies properly
The CI is run, so I can make sure that I'm not missing something super obvious
It allows people to give high-level feedback (i.e. I can upload a diff and bounce ideas off people)

EricWF · 2023-09-12T16:43:25Z

Drafts seem to prevent merging, not review.

Yes. It also prevents notifying people until it is made a PR.

Is there a way to filter pull requests to non-draft libc++ PR's? If not, then I think we need to reconsider the purpose of drafts.

-is:draft label:libc++

What is the purpose of opening the draft if you didn't want review?

It has multiple benefits:

People see what I'm working on and don't duplicate it

I can work on any comments etc. for the PR

It's a draft. You stated you don't want review comments yet. Are you going to address the comments I left?

I can easily view the diff and make sure that the patch applies properly

You can do that in your own branch.

The CI is run, so I can make sure that I'm not missing something super obvious

I'm going to be working to stop Draft PR from using shared resources. It's unfair to commits that are ready to go.

It allows people to give high-level feedback (i.e. I can upload a diff and bounce ideas off people)

Again, link to your own github fork.

Again, you either don't want feedback or you do. It's unclear from this conversation which it is, but it seems like you're having it both ways.

libcxx/benchmarks/algorithms/lexicographical_compare.bench.cpp

libcxx/include/__algorithm/lexicographical_compare.h

libcxx/include/__algorithm/ranges_lexicographical_compare.h

libcxx/include/__type_traits/desugars_to.h

libcxx/include/__algorithm/ranges_minmax.h

github-actions · 2024-07-28T12:13:06Z

✅ With the latest revision this PR passed the C/C++ code formatter.

ldionne

LGTM with my comments applied. Thanks, this is a neat optimization.

libcxx/include/__type_traits/desugars_to.h

libcxx/include/__algorithm/comp.h

libcxx/include/__algorithm/lexicographical_compare.h

libcxx/include/__algorithm/comp.h

If the comparison operation is equivalent to < and that is a total order, we know that we can use equality comparison on that type instead to extract some information. Furthermore, if equality comparison on that type is trivial, the user can't observe that we're calling it. So instead of using the user-provided total order, we use std::mismatch, which uses equality comparison (and is vertorized). Additionally, if the type is trivially lexicographically comparable, we can go one step further and use std::memcmp directly instead of calling std::mismatch. Benchmarks: ``` ------------------------------------------------------------------------------------- Benchmark old new ------------------------------------------------------------------------------------- bm_lexicographical_compare<unsigned char>/1 1.17 ns 2.34 ns bm_lexicographical_compare<unsigned char>/2 1.64 ns 2.57 ns bm_lexicographical_compare<unsigned char>/3 2.23 ns 2.58 ns bm_lexicographical_compare<unsigned char>/4 2.82 ns 2.57 ns bm_lexicographical_compare<unsigned char>/5 3.34 ns 2.11 ns bm_lexicographical_compare<unsigned char>/6 3.94 ns 2.21 ns bm_lexicographical_compare<unsigned char>/7 4.56 ns 2.11 ns bm_lexicographical_compare<unsigned char>/8 5.25 ns 2.11 ns bm_lexicographical_compare<unsigned char>/16 9.88 ns 2.11 ns bm_lexicographical_compare<unsigned char>/64 38.9 ns 2.36 ns bm_lexicographical_compare<unsigned char>/512 317 ns 6.54 ns bm_lexicographical_compare<unsigned char>/4096 2517 ns 41.4 ns bm_lexicographical_compare<unsigned char>/32768 20052 ns 488 ns bm_lexicographical_compare<unsigned char>/262144 159579 ns 4409 ns bm_lexicographical_compare<unsigned char>/1048576 640456 ns 20342 ns bm_lexicographical_compare<signed char>/1 1.18 ns 2.37 ns bm_lexicographical_compare<signed char>/2 1.65 ns 2.60 ns bm_lexicographical_compare<signed char>/3 2.23 ns 2.83 ns bm_lexicographical_compare<signed char>/4 2.81 ns 3.06 ns bm_lexicographical_compare<signed char>/5 3.35 ns 3.30 ns bm_lexicographical_compare<signed char>/6 3.90 ns 3.99 ns bm_lexicographical_compare<signed char>/7 4.56 ns 3.78 ns bm_lexicographical_compare<signed char>/8 5.20 ns 4.02 ns bm_lexicographical_compare<signed char>/16 9.80 ns 6.21 ns bm_lexicographical_compare<signed char>/64 39.0 ns 3.16 ns bm_lexicographical_compare<signed char>/512 318 ns 7.58 ns bm_lexicographical_compare<signed char>/4096 2514 ns 47.4 ns bm_lexicographical_compare<signed char>/32768 20096 ns 504 ns bm_lexicographical_compare<signed char>/262144 156617 ns 4146 ns bm_lexicographical_compare<signed char>/1048576 624265 ns 19810 ns bm_lexicographical_compare<int>/1 1.15 ns 2.12 ns bm_lexicographical_compare<int>/2 1.60 ns 2.36 ns bm_lexicographical_compare<int>/3 2.21 ns 2.59 ns bm_lexicographical_compare<int>/4 2.74 ns 2.83 ns bm_lexicographical_compare<int>/5 3.26 ns 3.06 ns bm_lexicographical_compare<int>/6 3.81 ns 4.53 ns bm_lexicographical_compare<int>/7 4.41 ns 4.72 ns bm_lexicographical_compare<int>/8 5.08 ns 2.36 ns bm_lexicographical_compare<int>/16 9.54 ns 3.08 ns bm_lexicographical_compare<int>/64 37.8 ns 4.71 ns bm_lexicographical_compare<int>/512 309 ns 24.6 ns bm_lexicographical_compare<int>/4096 2422 ns 204 ns bm_lexicographical_compare<int>/32768 19362 ns 1947 ns bm_lexicographical_compare<int>/262144 155727 ns 19793 ns bm_lexicographical_compare<int>/1048576 623614 ns 80180 ns bm_ranges_lexicographical_compare<unsigned char>/1 1.07 ns 2.35 ns bm_ranges_lexicographical_compare<unsigned char>/2 1.72 ns 2.13 ns bm_ranges_lexicographical_compare<unsigned char>/3 2.46 ns 2.12 ns bm_ranges_lexicographical_compare<unsigned char>/4 3.17 ns 2.12 ns bm_ranges_lexicographical_compare<unsigned char>/5 3.86 ns 2.12 ns bm_ranges_lexicographical_compare<unsigned char>/6 4.55 ns 2.12 ns bm_ranges_lexicographical_compare<unsigned char>/7 5.25 ns 2.12 ns bm_ranges_lexicographical_compare<unsigned char>/8 5.95 ns 2.13 ns bm_ranges_lexicographical_compare<unsigned char>/16 11.7 ns 2.13 ns bm_ranges_lexicographical_compare<unsigned char>/64 45.5 ns 2.36 ns bm_ranges_lexicographical_compare<unsigned char>/512 366 ns 6.35 ns bm_ranges_lexicographical_compare<unsigned char>/4096 2886 ns 40.9 ns bm_ranges_lexicographical_compare<unsigned char>/32768 23054 ns 489 ns bm_ranges_lexicographical_compare<unsigned char>/262144 185302 ns 4339 ns bm_ranges_lexicographical_compare<unsigned char>/1048576 741576 ns 19430 ns bm_ranges_lexicographical_compare<signed char>/1 1.10 ns 2.12 ns bm_ranges_lexicographical_compare<signed char>/2 1.66 ns 2.35 ns bm_ranges_lexicographical_compare<signed char>/3 2.23 ns 2.58 ns bm_ranges_lexicographical_compare<signed char>/4 2.82 ns 2.82 ns bm_ranges_lexicographical_compare<signed char>/5 3.34 ns 3.06 ns bm_ranges_lexicographical_compare<signed char>/6 3.92 ns 3.99 ns bm_ranges_lexicographical_compare<signed char>/7 4.64 ns 4.10 ns bm_ranges_lexicographical_compare<signed char>/8 5.21 ns 4.61 ns bm_ranges_lexicographical_compare<signed char>/16 9.79 ns 7.42 ns bm_ranges_lexicographical_compare<signed char>/64 38.9 ns 2.93 ns bm_ranges_lexicographical_compare<signed char>/512 317 ns 7.31 ns bm_ranges_lexicographical_compare<signed char>/4096 2500 ns 47.5 ns bm_ranges_lexicographical_compare<signed char>/32768 19940 ns 496 ns bm_ranges_lexicographical_compare<signed char>/262144 159166 ns 4393 ns bm_ranges_lexicographical_compare<signed char>/1048576 638206 ns 19786 ns bm_ranges_lexicographical_compare<int>/1 1.10 ns 2.12 ns bm_ranges_lexicographical_compare<int>/2 1.64 ns 3.04 ns bm_ranges_lexicographical_compare<int>/3 2.23 ns 2.58 ns bm_ranges_lexicographical_compare<int>/4 2.81 ns 2.81 ns bm_ranges_lexicographical_compare<int>/5 3.35 ns 3.05 ns bm_ranges_lexicographical_compare<int>/6 3.94 ns 4.60 ns bm_ranges_lexicographical_compare<int>/7 4.60 ns 4.81 ns bm_ranges_lexicographical_compare<int>/8 5.19 ns 2.35 ns bm_ranges_lexicographical_compare<int>/16 9.85 ns 2.87 ns bm_ranges_lexicographical_compare<int>/64 38.9 ns 4.70 ns bm_ranges_lexicographical_compare<int>/512 318 ns 24.5 ns bm_ranges_lexicographical_compare<int>/4096 2494 ns 202 ns bm_ranges_lexicographical_compare<int>/32768 20000 ns 1939 ns bm_ranges_lexicographical_compare<int>/262144 160433 ns 19730 ns bm_ranges_lexicographical_compare<int>/1048576 642636 ns 80760 ns ```

philnik777 added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. performance labels Sep 4, 2023

philnik777 force-pushed the optimize_lexicographical_compare branch 2 times, most recently from fec7ee6 to d28be44 Compare September 5, 2023 01:23

ldionne mentioned this pull request Sep 5, 2023

[libc++] Fix CODEOWNERS file for libc++ #65344

Closed

EricWF closed this Sep 6, 2023

philnik777 reopened this Sep 6, 2023

ldionne self-assigned this Sep 13, 2023

philnik777 force-pushed the optimize_lexicographical_compare branch 5 times, most recently from 68c2eb6 to 77c776a Compare July 18, 2024 14:48

ldionne requested changes Jul 25, 2024

View reviewed changes

philnik777 force-pushed the optimize_lexicographical_compare branch 2 times, most recently from e85cefc to 86967cb Compare July 28, 2024 12:09

philnik777 force-pushed the optimize_lexicographical_compare branch from 86967cb to 708f75a Compare July 31, 2024 17:50

philnik777 marked this pull request as ready for review July 31, 2024 17:51

philnik777 requested a review from a team as a code owner July 31, 2024 17:51

ldionne approved these changes Aug 1, 2024

View reviewed changes

philnik777 force-pushed the optimize_lexicographical_compare branch from 708f75a to d5dedbc Compare August 1, 2024 22:02

[libc++] Forward to memcmp in lexicographical_compare

dcd5f8a

philnik777 force-pushed the optimize_lexicographical_compare branch from d5dedbc to dcd5f8a Compare August 2, 2024 08:27

philnik777 merged commit d07fdf9 into llvm:main Aug 4, 2024
55 checks passed

philnik777 deleted the optimize_lexicographical_compare branch August 4, 2024 08:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc++] Optimize lexicographical_compare #65279

[libc++] Optimize lexicographical_compare #65279

philnik777 commented Sep 4, 2023 •

edited

Loading

EricWF commented Sep 5, 2023

EricWF commented Sep 5, 2023

EricWF commented Sep 5, 2023

EricWF commented Sep 5, 2023

philnik777 commented Sep 6, 2023

EricWF commented Sep 6, 2023

philnik777 commented Sep 6, 2023

EricWF commented Sep 7, 2023

philnik777 commented Sep 7, 2023

EricWF commented Sep 12, 2023

github-actions bot commented Jul 28, 2024 •

edited

Loading

ldionne left a comment

[libc++] Optimize lexicographical_compare #65279

[libc++] Optimize lexicographical_compare #65279

Conversation

philnik777 commented Sep 4, 2023 • edited Loading

EricWF commented Sep 5, 2023

EricWF commented Sep 5, 2023

EricWF commented Sep 5, 2023

EricWF commented Sep 5, 2023

philnik777 commented Sep 6, 2023

EricWF commented Sep 6, 2023

philnik777 commented Sep 6, 2023

EricWF commented Sep 7, 2023

philnik777 commented Sep 7, 2023

EricWF commented Sep 12, 2023

github-actions bot commented Jul 28, 2024 • edited Loading

ldionne left a comment

Choose a reason for hiding this comment

philnik777 commented Sep 4, 2023 •

edited

Loading

github-actions bot commented Jul 28, 2024 •

edited

Loading