Android: Use Hermes

[chrisbobbe]

This will be possible following the RN 60 upgrade, #3548.

The guide we'll want to follow is at https://reactnative.dev/docs/0.61/hermes (or https://reactnative.dev/docs/0.62/hermes if we're on RN v0.62 when we get to this), including all the steps to ensure it's enabled properly.

[gnprice]

I think this may also block on Hermes growing support for Intl:
facebook/hermes#23
and in particular growing enough support that react-intl works. The good news is that they seem to be working on that now.

[timabbott]

Interesting. We probably want to at least prototype running Hermes even if it breaks i18n, just to get a sense of what the performance impact and discover any other things we need to fix on our end.

[agrawal-d]

I'm very excited about this!

[felippepuhle]

You can import some polyfills for android, ex:

if (Platform.OS === 'android') {	
  require('intl')	
  require('intl/locale-data/jsonp/en')	

  require('date-time-format-timezone/build/src/code/polyfill.js').default(	
    global	
  )	
  require('date-time-format-timezone/build/src/code/data-loader.js').default(	
    global	
  )	
  require('date-time-format-timezone/build/src/data/metazone.js')(global)	
  require('date-time-format-timezone/build/src/data/locales/locale-en.js')(	
    global	
  )	
  require('date-time-format-timezone/build/src/data/timezones/tzdata-america-los_angeles.js')(	
    global	
  )	
}

We've used to do something like this in the past but we're currently using react-native-v8.

[gnprice]

but we're currently using react-native-v8.

@felippepuhle Interesting! I hadn't been really aware of react-native-v8. I just looked around on the Web and found some benchmark reports, and it seems like react-native-v8 makes different tradeoffs from Hermes.

I'd be very curious to hear more, if you'd be up for sharing, about why you switched from Hermes to react-native-v8 🙂 . Either on this thread, or on #mobile on chat.zulip.org, which might be a more convenient venue for discussion.

[chrisbobbe]

Hmm, I'm getting a "Too many handles allocated in GCScope" error after changing enableHermes from false to true, adding the line in proguard-rules.pro, and running ./gradlew clean. It resembles facebook/react-native#25730 and facebook/hermes#26; looks like I should look into attaching a native debugger like gdb or lldb. I'm curious what this would get me that adb logcat doesn't.

Continuing the discussion here.

[chrisbobbe]

Awaiting a response to a React Native issue; see discussion here. This will no longer block the RN v0.61 upgrade.

[mhorowitz]

"Too many handles allocated in GCScope" is an internal issue in Hermes. I don't know what version you're using, but if you can file an issue on the hermes repo with as much repro information as you can, we can look into it. This error is usually easy to debug with a native stack trace.

[chrisbobbe]

Looks like we can expect Hermes to have Intl support with the React Native v0.65 release: facebook/hermes#23 (comment)

[chrisbobbe]

There's an issue in RN v0.65 that I think affects apps using Hermes on Android, and it's reportedly fixed in RN v0.66: facebook/react-native#32197 (comment).

#5372 is open for the RN v0.66 upgrade.

[gnprice]

I'm taking a look at this now. (It looks like it'll be helpful for #5589 .)

The first obstacle I hit is a crash that appears to be facebook/hermes#602. That was fixed in RN v0.68, for which our upgrade issue is #5344.

There's a suggested workaround by monkey-patch on that thread. I may try that in order to unblock Hermes; but if we do switch, that will probably add a reason to catch back up on RN upgrades.

[gnprice]

There's a suggested workaround by monkey-patch on that thread.

This didn't suffice. It worked as far as it goes -- it appeared to fix String#localeCompare so it doesn't crash on the empty string. But the app still crashes after the loading screen, i.e. presumably when it has server data ready and starts trying to process it and/or render the main UI.

Reading a bit more closely the Hermes commit that fixed the issue, it seems clear that a number of other Intl-related methods are affected. There doesn't seem to be a good way of identifying which of them is triggering the issue; there may also not be workarounds possible for some of them that are as straightforward as that one for String#localeCompare. So I don't think the monkey-patch approach will work for us; we'll need to take care of the RN upgrade #5344 before we can return to trying Hermes on Android.

[gnprice]

Completed that RN v0.68 upgrade #5344 yesterday; picking this back up again.

[gnprice]

Update today:

• I have a draft branch to switch to Hermes, where everything seems to work 🎉
  • It has a bunch of prep commits, followed by the main commit making the change. The prep commits are all mergeable.
  • One thing they take care of is that there are still some bits of Intl Hermes doesn't supply, so we need to polyfill those. Fortunately I think these are small.
  • The other is how the debugging situation changes. Some discussion here. I think using redux-logger will get slightly more annoying, but still be totally usable (after the changes in the branch.)
• However, the perf is not great.
  • Most visibly, if you hit the "New PM" button (on chat.zulip.org), it takes a long time to respond — like tens of seconds.
    • This is because we're sorting the users by name in UserList, and that turns out to get very slow in Hermes.
    • I isolated an especially sharp piece of that — some of the individual String#localeCompare calls take multiple seconds each — and reported it upstream as localeCompare extremely slow sometimes facebook/hermes#867 .
    • There are certainly things we can do to make that component more algorithmically efficient. But when making enough comparisons to sort the whole list just once takes like 27 seconds, I think any clever algorithm is still going to be painfully slow.
    • One crude workaround would be to drop the use of String#localeCompare and just compare the strings' Unicode codepoints numerically (or do that with the results of String#toLowerCase). That produces pretty tolerable results in English and I think most Latin-letter languages, at least.
  • I'm not sure how widespread the perf issues are beyond that.