-
Notifications
You must be signed in to change notification settings - Fork 391
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
Implement support for dylib linkage #2414
Conversation
82df1a2
to
7148f30
Compare
7148f30
to
a4e0d62
Compare
9922078
to
64c5a12
Compare
64c5a12
to
3d17f40
Compare
ef1a896
to
e324956
Compare
e324956
to
b763eef
Compare
This reverts commit 99740e8.
5e115e5
to
d4cc02e
Compare
65c0d69
to
1b394da
Compare
a537a94
to
1307f9a
Compare
1307f9a
to
c3d1fb7
Compare
c3d1fb7
to
4f92e5f
Compare
@illicitonion I added analysis tests and fixed the flag name. PTAL. The tests fail on window and darwin for not having Does anyone know why? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid nothing jumps out about why the tests are failing, but happy to spend a bit of time debugging if no one else has any insights!
9addf7f
to
630cd1d
Compare
630cd1d
to
baef75a
Compare
This PR implements dylib linkage against the standard library behind a feature flag `--@rules_rust//rust/settings:experimental_use_dylib_linkage`. The main part of this feature is [here](https://github.com/bazelbuild/rules_rust/pull/2414/files#diff-2a806da393e47c07ffe67c78ace69eb488b4ac44b029a46d8237b8e2a05637beR258) where we skip exporting static rust stdlibs and export only `libstd.so` instead. This feature is useful when the subset of libstd being statically linked to downstream shared libraries and binaries is **larger** than the entire dylib version of libstd. The following diagram is the high level of what dylib linkage is trying to achieve. ![Untitled Diagram drawio](https://github.com/bazelbuild/rules_rust/assets/13268391/d19f18f5-c2d1-4ddc-b170-773a6004f732) Running the feature against `android_binary` yields a size reduction on the shared library produced by `android_binary` because it doesn't statically link the rust stdlibs anymore. ``` > bazel build //:android_app --config=android_x86_64 > unzip -l bazel-bin/android_app.apk Archive: bazel-bin/android_app.apk Length Date Time Name --------- ---------- ----- ---- 1381968 2010-01-01 00:00 lib/x86_64/libandroid_app.so <--- static link with rust stdlibs --------- ------- 1390294 9 files ``` ``` > bazel build //:android_app --config=android_x86_64 --config=dylib_linkage > unzip -l bazel-bin/android_app.apk Archive: bazel-bin/android_app.apk Length Date Time Name --------- ---------- ----- ---- 8080 2010-01-01 00:00 lib/x86_64/libandroid_app.so <--- reduced size because of dynamic linking 13055776 2010-01-01 00:00 lib/x86_64/libstd-8d416d49cf02ecea.so --------- ------- 13072400 10 files ``` Here, the benefit comes when there are enough shared libraries statically linking against the rust stdlibs. "Enough" here means that the total up size of those libraries being more than just the entire `libstd.so`. TODO: I'm leaving this PR without unit tests until I get some feedback or suggestions on my approach. --------- Co-authored-by: scentini <rosica@google.com>
This PR implements dylib linkage against the standard library behind a feature flag
--@rules_rust//rust/settings:experimental_use_dylib_linkage
.The main part of this feature is here where we skip exporting static rust stdlibs and export only
libstd.so
instead.This feature is useful when the subset of libstd being statically linked to downstream shared libraries and binaries is larger than the entire dylib version of libstd. The following diagram is the high level of what dylib linkage is trying to achieve.
Running the feature against
android_binary
yields a size reduction on the shared library produced byandroid_binary
because it doesn't statically link the rust stdlibs anymore.Here, the benefit comes when there are enough shared libraries statically linking against the rust stdlibs. "Enough" here means that the total up size of those libraries being more than just the entire
libstd.so
.TODO: I'm leaving this PR without unit tests until I get some feedback or suggestions on my approach.