[libcxx-commits] [PATCH] D113413: Add introsort to avoid O(n^2) behavior and a benchmark for adversarial quick sort input.

Wed May 18 13:22:46 PDT 2022

ayzhao added a comment.

In D113413#3516599 <https://reviews.llvm.org/D113413#3516599>, @pkasting wrote:

> In D113413#3512743 <https://reviews.llvm.org/D113413#3512743>, @ayzhao wrote:
>
>> We see that this patch is causing the size of the Chrome Android APK to increase by ~120 KB <https://crbug.com/1324722#c2>. This is confirmed via bisection. A breakdown of the symbols contributing to the size increase can be found at https://chrome-supersize.firebaseapp.com/viewer.html?load_url=https%3A%2F%2Fstorage.googleapis.com%2Fchromium-binary-size-trybot-results%2Fandroid-binary-size%2F2022%2F05%2F10%2F1112911%2Fsupersize_diff.sizediff&group_by=template .
>>
>> At this point in time, I'm not exactly sure why this patch would increase the size by such a large amount. Does anyone have any ideas?
>
> @From the symbol diffs on e.g. ng_grid_layout_algorithm.cc, it looks like we have lots of negative deltas for __sort and larger positive deltas for __introsort, which suggests that the difference is the cumulative effect of inlining many instances of __introsort with its slightly longer (len > 5) case in the new code.
>
> Perhaps this code is being built with excessive inlining and needs -Os or similar?  Perhaps Clang itself needs some inliner heuristic tuning?  Perhaps this is an unavoidable size hit unless you want to take a speed hit?

For documentation purposes:

I chatted offline with pkasting@ and we found out that adding `__attribute__((noinline))` to `__introsort` actually made the binary size even worse - it added an additional ~40KB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113413/new/

https://reviews.llvm.org/D113413