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

Implement support for dylib linkage #2414

Merged
merged 43 commits into from
Feb 23, 2024

Conversation

daivinhtran
Copy link
Contributor

@daivinhtran daivinhtran commented Jan 10, 2024

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.

Untitled Diagram drawio

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.

@daivinhtran daivinhtran marked this pull request as draft January 10, 2024 19:33
@daivinhtran daivinhtran force-pushed the link-std-dylib branch 2 times, most recently from 82df1a2 to 7148f30 Compare January 10, 2024 19:40
@daivinhtran daivinhtran force-pushed the link-std-dylib branch 2 times, most recently from ef1a896 to e324956 Compare January 17, 2024 06:04
examples/android/BUILD.bazel Outdated Show resolved Hide resolved
@daivinhtran daivinhtran changed the title Implementing option to link std dynamically for target build Implement support for dylib linkage Feb 19, 2024
@daivinhtran daivinhtran force-pushed the link-std-dylib branch 2 times, most recently from a537a94 to 1307f9a Compare February 23, 2024 05:07
@daivinhtran daivinhtran marked this pull request as ready for review February 23, 2024 13:14
@daivinhtran
Copy link
Contributor Author

daivinhtran commented Feb 23, 2024

@illicitonion I added analysis tests and fixed the flag name. PTAL.

The tests fail on window and darwin for not having prefer-dynamic set here. For some reason, it only happens on window and darwin. The test overrides the flag explicitly.

Does anyone know why?

Copy link
Collaborator

@illicitonion illicitonion left a 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!

rust/toolchain.bzl Outdated Show resolved Hide resolved
test/link_std_dylib/link_std_dylib_test.bzl Outdated Show resolved Hide resolved
~ Outdated Show resolved Hide resolved
@daivinhtran daivinhtran added this pull request to the merge queue Feb 23, 2024
Merged via the queue into bazelbuild:main with commit e7f5516 Feb 23, 2024
3 checks passed
qtica added a commit to qtica/rules_rust that referenced this pull request Apr 1, 2024
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants