[libc++] Optimize lexicographical_compare

[philnik777]

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]

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]

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

[EricWF]

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]

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

[philnik777]

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]

@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]

@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]

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]

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]

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.

[github-actions]

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

[ldionne]

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