Optimize extraction strategy #45
Conversation
There was a problem hiding this comment.
@oprisnik has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
codyi96
force-pushed
the
bugfix/optimizeExtractionStrategy
branch
from
e5b7957
to
33ee0cd
Compare
|
@codyi96 has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@oprisnik has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
Hi, thanks for taking a look at this!
Let me see if I understand the problem correctly:
Oppo/Vivo devices are reporting 64-bit ABIs in Build.CPU_ABI/SUPPORTED_ABIS but instead extract the 32-bit libraries, presumably to improve RSS and general performance of the device. Correct?
The approach you're taking here (parsing already-extracted libraries) is correct but also a little excessive.
Assuming that these phones are not mixing up cpu families (x86 vs arm), the following logic is far simpler:
- Determine CPU family
- Determine process bitness
- Load correct ABI based on cpu+bitness
I assume family is correct in Build.CPU_ABI, so let's use that (libhoudini comes to mind but whatever, it's mostly a relic of the past). Bitness is also trivial to compute:
// Check to see if we're actually running in 64-bit mode.
try {
return android.system.Os.readlink("/proc/self/exe").contains("64");
} catch (android.system.ErrnoException e) {
return false;
}
This is not only easier to understand but also faster as you only need to resolve a symlink.
I don't have access to one of these devices to try it but I sincerely doubt Oppo messed with the app_process vs app_process64 naming.
So far I'm talking about the issue in the bug report this PR refers to (bitness being incorrect). I can however a see a slightly different issue as well - SoLoader should be falling back to system-extracted libraries automatically, it prepends ApplicationSoSource to its search list.
Is SoLoader not finding the system-extracted libs for some reason or are the react libs packaged in a way that is not system-installer-visible? (Sorry, don't know much about react native)
P.S. minor code style/organization things inline.
|
@codyi96 has updated the pull request. Re-import the pull request |
codyi96
force-pushed
the
bugfix/optimizeExtractionStrategy
branch
from
9a7ac10
to
dc3025a
Compare
|
@codyi96 has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
Awesome, thanks @codyi96, this is really clean! 🥇
I imagine you retested this and it's still fine?
Left a couple of a minor comments inline. Do you mind addressing them, squashing into one commit and cleaning up the ELF code so we can land it cleanly?
codyi96
force-pushed
the
bugfix/optimizeExtractionStrategy
branch
from
dc3025a
to
f09c8b6
Compare
|
@codyi96 has updated the pull request. Re-import the pull request |
codyi96
force-pushed
the
bugfix/optimizeExtractionStrategy
branch
from
f09c8b6
to
9553342
Compare
|
@codyi96 has updated the pull request. Re-import the pull request |
Fixed: facebook/react-native#25490 Make extraction strategy of .so files smarter. In past versions, SoLoader follows standard rule to extract .so files from zip. However, some phones customize application installer which doesn't follow standard rule. It may cause crash because .so files of different ABIs are loaded in the application. In order to deal with this problem, SoLoader will parse the path of current process to determine which ABI should be extracted. It'll ensure that only one type of ABI will be loaded in the application so that SoLoader can be adapted for more models.
codyi96
force-pushed
the
bugfix/optimizeExtractionStrategy
branch
from
9553342
to
44956a2
Compare
|
@codyi96 has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@oprisnik has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Is code being reviewed? @oprisnik |
|
Yes, we're on it. Should be merged soon. Fingers crossed :) |
|
Still seeing incorrect version of lib being loaded and can reproduce it reliably on emulator: #55 |
fix bug.
Make extraction strategy of .so files smarter.
In past versions, SoLoader follows standard rule to extract .so files from zip. However, some phones customize application installer which doesn't follow standard rule. And then, may cause crash because .so files of different ABI is loaded in the application.
In this version, SoLoader will parse .so files in lib dir to determine which ABI of .so files should be extracted to lib-main dir. It can ensure that only one type of ABI will be loaded in the application so that SoLoader can be adapted for more models.