Skip to content
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

Find the correct way to package libicu data files #8

Closed
zhuowei opened this issue Dec 17, 2015 · 10 comments
Closed

Find the correct way to package libicu data files #8

zhuowei opened this issue Dec 17, 2015 · 10 comments

Comments

@zhuowei
Copy link
Member

zhuowei commented Dec 17, 2015

Swift needs at least the collation data; a full libicudata.so is like 25MB; need to find better solution for packaging it...

Swift needs something in the icudata, anyways, since for MakeRootCollator() without data it dies after

newfstatat(AT_FDCWD, "./share/icu/55.1/icudt55l/coll/ucadata.icu", 0xbed26088, 0) = -1 ENOENT (No such file or directory)
brk(0x41268000)                         = 0x41268000
futex(0x40f15054, FUTEX_WAKE_PRIVATE, 2147483647) = 0
newfstatat(AT_FDCWD, "./share/icu/55.1/icudt55l.dat", 0xbed26058, 0) = -1 ENOENT (No such file or directory)
@zhuowei zhuowei added the bug label Dec 17, 2015
@zhuowei zhuowei changed the title Illegal instruction errors when running through test suite Unreachable breakpoint opcode triggered in MakeRootCollator(), resulting in "Illegal instruction" when running through test suite Dec 18, 2015
@zhuowei zhuowei changed the title Unreachable breakpoint opcode triggered in MakeRootCollator(), resulting in "Illegal instruction" when running through test suite ""ucol_open: Failure setting up default collation." assert in MakeRootCollator resulting in "Illegal instruction" when running through test suite Dec 18, 2015
@zhuowei zhuowei changed the title ""ucol_open: Failure setting up default collation." assert in MakeRootCollator resulting in "Illegal instruction" when running through test suite Find the correct way to package libicu data files Dec 18, 2015
@zhuowei zhuowei added enhancement and removed bug labels Dec 18, 2015
@ephemer
Copy link
Member

ephemer commented Dec 18, 2015

I wondered whether static linking could fix the issue of size, or simply including the relevant source files in the Swift source bundle for certain platforms (I gather Android isn't the only one without built-in libicu support). I don't know how deep the rabbit hole of internal dependencies goes, but we may be able to get away with just compiling a small subset of libicu source files directly alongside the stdlib (removing the external and huge library dependency).

@ephemer
Copy link
Member

ephemer commented Dec 21, 2015

Android seems to come with a precompiled libicu, at least icui18n and icuuc, which seem to be the modules required by the swift stdlib. These files are each about 1.5mb or less on my Galaxy Tab. Don't know if we can use these pre-compiled binaries in any reasonable way, or what the implications of them not being included in the NDK are.. I still can't get them or the self-compiled libraries to link on an actual device (seems to work on the emulator though) – just getting missing symbol errors as if the library was never bundled at all.

I think the extra size for the compiled libicu modules in SwiftAndroid can be accounted for by their debugging symbols. They can be stripped via $NDK_ROOT/toolchains/arm-linux-androideabi-4.8/prebuilt/linux-x86_64/arm-linux-androideabi/bin/strip – this reduced the resulting binary size by up to 90% in my tests

@ephemer
Copy link
Member

ephemer commented Dec 21, 2015

I also noticed there is an intermediate coll.o file hanging around somewhere in the build directory. Maybe it'd be enough to just the stdlib link against it directly at compile time? Again I'm out of my depth there though

@zhuowei
Copy link
Member Author

zhuowei commented Dec 22, 2015

I do strip the libicu in the release tar.xzs and it doesn't make too much of a difference. We can probably statically link libicu but not sure of the license implications of that (Swift stdlib is Apache 2 with Runtime Exception)

I'll rather not depend on the Android system libicu since it's not a NDK library, so Android isn't required to have it or maintain compat - and I heard that phone manufacturers do customize the library to remove/add language support as needed, which would cause a lot of problems.

@ephemer
Copy link
Member

ephemer commented Dec 23, 2015

Good points. Yes, I saw today that Android Studio automatically strips the
dlibs when packaging an APK.

In any case I still haven't got any further with the linking problem. I
wonder if ICU is depending on some other libraries that are only present on
the emulator.

I'm trying to find a good way to debug the loadLibrary errors via readelf
strace and nm (any other tips and tools welcome), currently left with more
questions than when I began though.

At the moment I'm trying to track down another error whereby (even on the
emulator) my C library can't seem to properly link a custom Swift library,
even though the swift library loads fine directly via loadLibrary in that
environment.
zhuowei [email protected] schrieb am Di., 22. Dez. 2015 um 19:46:

I do strip the libicu in the shipped versions and it doesn't make too much
of a difference. We can probably statically link libicu but not sure of the
license implications of that (Swift stdlib is Apache 2 with Runtime
Exception)

I'll rather not depend on the Android system libicu since it's not a NDK
library, so Android isn't required to have it or maintain compat - and I
heard that phone manufacturers do customize the library to remove/add
language support as needed, which would cause a lot of problems.


Reply to this email directly or view it on GitHub
#8 (comment).

@ephemer
Copy link
Member

ephemer commented Dec 23, 2015

I think the issue is that my device has an old version of ICU pre-installed in /system/lib (see http://stackoverflow.com/questions/34441104/android-ndk-resolve-dynamic-library-version-conflict for a full write-up) I don't know enough about symbol resolution to be sure though – it seems weird that even if I load ICU manually, the symbols are still missing. The emulator seems to have a newer ICU installed in /system/lib, so it "just works".

I also asked the mailing list about statically linking ICU into the stdlib, the response was that it would be a non-trivial change to the CMake files. I'm starting to think we're running out of other options though, unless someone comes up with a good response to the stack overflow question. I'm enjoying learning about this but I'm still very much over my depth here. I'd rather be working on JNI wrappers or something else a bit higher-level, but not much I can do until this issue is out of the way.

Out of interest, have you got any Swift APKs working on a non-rooted device?

@ephemer
Copy link
Member

ephemer commented Dec 28, 2015

You can reduce icudata size with this tool (see bottom for tools supporting various ICU versions)
http://apps.icu-project.org/datacustom/

@ewmailing
Copy link

Hi, I have information that will benefit your effort. I had to build ICU as a dependency for JavaScriptCore on Android and hit very similar issues. Ultimately, I went with static linking. I documented how to do that here:

https://github.com/appcelerator/hyperloop/wiki/Building-JavaScriptCore-for-Android
Also, I wrote automation scripts for an internal Jenkins server here:
https://github.com/appcelerator/hyperloop/wiki/Building-JavaScriptCore:-Jenkins-Scripts

I think static linking is the correct way to go due to some of the problems you discovered. But the more concerning issue is since ICU is a private system library on Android, there is a potential for symbol name clashes or the internal Android library loader to reject your request to load your copy of the library (since it already exists). Incompatible versions or hacks could cause really big problems. Additionally, since you are middleware (i.e. people will be building apps with your stuff), there is the possibility your users need ICU for their own stuff and will bring in yet another (conflicting) copy of ICU. Static linking avoids this mess.

The big downside is that the binary size is huge. As you also noticed, the data set is a large piece of that bloat. And as already linked to, you can customize the data set to strip out some things that you don't need.

However, I never fully finished investigating this for JavaScriptCore. I had just started playing with
--with-data-packaging=archive which additionally moves out the data set into a separate file instead of being part of the library. If you can use this, you get an additional size benefit for multi-arch packages since the data set won't bloat up each arch copy of the library.
I think in the JSC case, I think I was able to throw out everything for my test cases.

One thing I never experimented with was actually needing to load the data archive. I don't know where ICU expects to load it from (does it need to be in the APK or somewhere else?). But from a simplicity standpoint, I like not having to think about packaging because build systems are a pain. So figuring out how to statically link with a the minimal data set instead of a separate file is probably the ideal even if there is a little more code bloat due to multi-arch.

I hope this information helps you. I strongly recommend static linking ICU because of all the potential conflict issues.

@liufsd
Copy link

liufsd commented Apr 15, 2016

libicudata.so crash
SwiftAndroid/swift-android-samples#1

@modocache
Copy link
Member

Let's continue this discussion here: https://bugs.swift.org/browse/SR-1359

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants