Optimize UnicodeICU localeCompare

[sujankh]

Summary

This change eliminates repeated calls to ucol_open and ucol_close in hermes::platform_unicode::localeCompare for HERMES_PLATFORM_UNICODE_ICU. On a sample sort.js(that sorted 30k strings using localeCompare), the runtime goes down from ~500 ms to ~120ms.

Before:
I observed in my trace that ucol_open and ucol_close was called in every localeCompare invocation and each ucol_open took around 500-900 ns. ucol_close is tiny ~100ish ns in each call.
Note that the first call to ucol_open, it takes around 100us even after applying this diff.

Here is localeCompare call stack before the patch:

After
After making UCollator construction static, we avoid repeated ucol_open and ucol_close(except during static initialization) thus making the sort.js sorting fast.
Here is the localeCompare stack after the patch:

Looking at the code, i found out that HERMES_PLATFORM_UNICODE_ICU is not used in Android/iOS but can make desktop usage faster (it it useful??).

Here is the sort.js script which i got from one of the issues/posts in hermes.

print("gen...");
  const randInt = (end, start = 0) =>
    Math.floor(Math.random() * (end - start) + start);
  const alphabet = 'abcdefghijklmnopqrstuvwxyz';
  const names = Array.from({length: 30000}, () =>
     Array.from({length: 8}, () => alphabet[randInt(alphabet.length)]).join(''),
   );

  for(var i = 0; i < 5; ++i){
    print(names[i])
  }
  print('sorting...');
  const s = Date.now();
  names.sort(localeCompare);
  print(`...done, took ${Date.now() - s}ms`);

  function localeCompare(a, b) {
    const s = Date.now();
    const result = a.localeCompare(b);
    const e = Date.now();
    if (e - s > 1) {
      print(`slow localeCompare: ${e - s}ms for '${a}' vs '${b}'`);
    }
    return result;
  }

  for(var i = 0; i < 5; ++i){
    print(names[i])
  }

Test Plan

Execute sort.js before applying this patch

hermes (main)$ ../build_release/bin/hermesc -emit-binary -out sort.hbc samples/sort.js
hermes (main)$ ../build_release/bin/hvm ./sort.hbc 
gen...
sorting...
...done, took 466ms
hermes (main)$ ../build_release/bin/hvm ./sort.hbc 
gen...
sorting...
...done, took 486ms

Execute sort.js after applying this patch

$ ../build_release/bin/hermesc -emit-binary -out sort.hbc samples/sort.js
hermes (icu_localecompare_optimize)$ ../build_release/bin/hvm ./sort.hbc 
gen...
sorting...
...done, took 119ms
hermes (icu_localecompare_optimize)$ ../build_release/bin/hvm ./sort.hbc 
gen...
sorting...
...done, took 127ms

Run check-hermes:

hermes (icu_localecompare_optimize)$ cmake --build ../build_release/ --target check-hermes
[0/1] Running the Hermes regression tests
Testing Time: 15.91s
  Expected Passes    : 1773
  Unsupported Tests  : 66
hermes (icu_localecompare_optimize)$ echo $?
0

[facebook-github-bot]

Hi @sujankh!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[neildhar]

Thank you for working on this, I just have some minor comments.

[neildhar]

I don't believe the deleter is invoked if the pointer is null.

[sujankh]

Fixed. Thanks, re-read cppreference to make sure.

[neildhar]

LGTM, thanks for working on this!

[facebook-github-bot]

@neildhar has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@neildhar merged this pull request in 16a6e2c.