-
Notifications
You must be signed in to change notification settings - Fork 149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade Android toolchain #615
Conversation
Seems to work :)
Hey @shyfire131 , awesome that you are tackling this one. I tried a couple of times in the past and this task ended up giving me nightmares :D EDIT: disregard this one, the dev environment was not well set up.
When running
I cleared the cache of our android release workflow and retried, but it got the same error (cleaned everything too locally). |
Okay. I do not often quote Elon Musk but this is an exceptional case: "the best part, is no part". I removed Which leads to question why these dependencies were needed in the first place 😄 . I believe Find build artifacts on pre-release: https://github.com/RoboSats/robosats/releases/tag/android-66eb18c |
I checked out the main...update-android-toolchain branch locally and can confirm that removing v8 works on my side too. Although I think there is still a stale maven repository reference for it but I'll omit that when I update this branch. Going to gulp try bumping Gradle to Gradle 8 now. |
To be on the clear about the effect of removing V8 in performance, I wrote a small component that renders our webapp ResultsHere the summary stats of the 20 runs on a Pixel 6
The two-tailed P value = 0.0061 And 10 runs on a Pixel 3XL
The two-tailed P value = 0.0027 So...Looking at the mean rendering times, there is really no performance difference in absolute terms, times are very similar. We can safely drop V8 dependencies. However, the performance is indeed statisticallt different (positive test). Just not the way we expected, worse without V8, but the other way around: react-native 0.71 without V8 is a tiny faster (but who knows why, maybe other factors can explain, like phone temperature, background load, etc. But it is consistent across devices). Indeed, it seems the main factor for JS performance is the Webview engine of the Android system, which in GrapheneOS is Vanadium (i.e., Chromium, so it has V8). |
Very nice! Just to be clear, the null hypothesis for your p-values is "Removing V8 would slow down performance" ? |
Given that is a two-sided t-test, the null hypothesis is "Removing V8 has no performance impact". A small p-value, like we got, means that, indeed, removing V8 has a performance impact. However, the significant impact seems to be for increased performance with no V8! Certainly unexpected. Most likely V8 has no impact at all and the increased performance must be due to some optimizations of react-native v0.71.8. But that's enough of nerding around! :D |
Merged into main via another branch without the dependaBot merge conflicts. Thank you @shyfire131 taking a look and getting this to work on the newer react-native. Please accept a 200K Sats donation from the devfund. Send an invoice from a proxy wallets with a >24h expiration time :D |
lnbc2m1pj8vt9app5g77zcsphuzd8v0dlvqnmydaevgjflzfsz8agsgfjvu4ye2dtl2zqdqu2askcmr9wssx7e3q2dshgmmndp5scqzzsxqyz5vqsp5p76lkcy38564kypyk3gs47wxd8vvsjvugv2r5g0uvvzdqa7n9mes9qyyssqqpgtfyjtaqqs477r40dyvj2rj0ywlr9zx0hmljvkhatuahlyk4vjkv5hy0e0pad7txp89vnatdrnxl8z9gvehvvyfq8xpatdrcxu9cgpwwt9v6 |
f495d79433c47c6748cf1f13b81d4b4880a4b2bdded2e5038ec50cf73e667a1f |
Should buy android devs some time. Still want to investigate upgrading to Gradle 8.
Some notable upgrades:
Only tested on Pixel 7 up to seeing the order book, so needs more testing.
The diff of these files is the best place to see what was upgraded:
Generally things seem to work though :)
TODO:
Checklist before merging
pip install pre-commit
, thenpre-commit install
. Pre-commit installs git hooks that automatically check the codebase. If pre-commit fails when you commit your changes, please fix the problems it points out.