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.

Sign up for GitHub

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

Jump to bottom

set has_thread_local=true for android #96317

Closed
wants to merge 1 commit into from
Closed

set has_thread_local=true for android #96317

wants to merge 1 commit into from

Conversation

name1e5s
Copy link
Contributor

It seems that llvm uses emulated tls on android by default since this commit. Which uses a pthread_key to emulate various tls objects at runtime(code).

I've built a demo with a custom android target setting has_thread_local=true and a slightly modified std as what this PR does, and it works fine. The generated function symbol __emutls_get_address would link to an implementation in libgcc.a , which is also required by std for now.

By enable has_thread_local, we can reduce the number of pthread_keys used by rust libraries on android, which are quite limited resources(128 per process). I've been investgating some crashes caused by unable to create more pthread_key in our project with about 80 pthread_keys used by rust part.

cc #96145 rust-lang/compiler-builtins#458

@rust-highfive
Copy link
Collaborator

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 22, 2022
@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 22, 2022
@Mark-Simulacrum
Copy link
Member

We'll definitely want to remove the merge commit (please rebase it away).

Otherwise, this seems OK in general but I don't have sufficient context here. r? @Amanieu since you merged the compiler-builtins PR.

@Amanieu
Copy link
Member

Amanieu commented Apr 25, 2022

LGTM!

In general we should prefer using LLVM-backed TLS where possible since it can be optimized better.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 25, 2022

📌 Commit 73abc78 has been approved by Amanieu

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 25, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 25, 2022
@matthiaskrgr
Rollup merge of rust-lang#96317 - name1e5s:android_tls_on_1, r=Amanieu
579d86a 
set has_thread_local=true for android

It seems that llvm uses emulated tls on android by default since [this commit](https://reviews.llvm.org/D42999). Which uses a `pthread_key` to emulate various tls objects at runtime([code](https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/emutls.c)).

I've built a demo with a custom android target setting `has_thread_local=true` and a slightly modified `std` as what this PR does, and it works fine. The generated function symbol `__emutls_get_address`  would link to an implementation in `libgcc.a` , which is also [required by std](https://github.com/rust-lang/rust/blob/master/library/unwind/build.rs#L18) for now.

By enable `has_thread_local`, we can reduce the number of `pthread_key`s used by rust libraries on android, which are quite limited resources(128 per process). I've been investgating some crashes caused by unable to create more pthread_key in our project with about 80 `pthread_key`s used by rust part.

cc rust-lang#96145  [rust-lang/compiler-builtins#458](rust-lang/compiler-builtins#458)
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 25, 2022
Closed
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 25, 2022
@GuillaumeGomez
Rollup merge of rust-lang#96317 - name1e5s:android_tls_on_1, r=Amanieu
56a0418 
set has_thread_local=true for android

It seems that llvm uses emulated tls on android by default since [this commit](https://reviews.llvm.org/D42999). Which uses a `pthread_key` to emulate various tls objects at runtime([code](https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/emutls.c)).

I've built a demo with a custom android target setting `has_thread_local=true` and a slightly modified `std` as what this PR does, and it works fine. The generated function symbol `__emutls_get_address`  would link to an implementation in `libgcc.a` , which is also [required by std](https://github.com/rust-lang/rust/blob/master/library/unwind/build.rs#L18) for now.

By enable `has_thread_local`, we can reduce the number of `pthread_key`s used by rust libraries on android, which are quite limited resources(128 per process). I've been investgating some crashes caused by unable to create more pthread_key in our project with about 80 `pthread_key`s used by rust part.

cc rust-lang#96145  [rust-lang/compiler-builtins#458](rust-lang/compiler-builtins#458)
@GuillaumeGomez GuillaumeGomez mentioned this pull request Apr 25, 2022
Closed
@GuillaumeGomez
Copy link
Member

From #96380 (comment):

@bors: r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 25, 2022
@bors
Copy link
Contributor

bors commented May 6, 2022

☔ The latest upstream changes (presumably #96510) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon
Copy link
Member

ping from triage:
@name1e5s - can you please address the merge conflicts?

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

@JohnCSimon
Copy link
Member

@name1e5s
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with the PR.
Note: if you do please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Jun 19, 2022
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Jun 19, 2022
@quininer quininer mentioned this pull request Nov 13, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 29, 2023
@bors
Auto merge of rust-lang#117873 - quininer:android-emutls, r=Amanieu
b4cb52a 
Use emulated TLS for android

This is a reopen of rust-lang#96317 . many android devices still only use 128 pthread keys, so using emutls can be helpful.

r? `@Amanieu`
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 29, 2023
@bors
Auto merge of rust-lang#117873 - quininer:android-emutls, r=Amanieu
d638ba2 
Use emulated TLS for android

This is a reopen of rust-lang#96317 . many android devices still only use 128 pthread keys, so using emutls can be helpful.

r? `@Amanieu`
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 29, 2023
@bors
Auto merge of rust-lang#117873 - quininer:android-emutls, r=Amanieu
fd050b8 
Use emulated TLS for android

This is a reopen of rust-lang#96317 . many android devices still only use 128 pthread keys, so using emutls can be helpful.

r? `@Amanieu`
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2023
@bors
Auto merge of rust-lang#117873 - quininer:android-emutls, r=Amanieu
395d2e4 
Use emulated TLS for android

This is a reopen of rust-lang#96317 . many android devices still only use 128 pthread keys, so using emutls can be helpful.

r? `@Amanieu`
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 9, 2023
@bors
Auto merge of rust-lang#117873 - quininer:android-emutls, r=Amanieu
608f324 
Add emulated TLS support

This is a reopen of rust-lang#96317 . many android devices still only use 128 pthread keys, so using emutls can be helpful.

Currently LLVM uses emutls by default for some targets (such as android, openbsd), but rust does not use it, because `has_thread_local` is false.

This commit has some changes to allow users to enable emutls:

1. add `-Zhas-thread-local` flag to specify that std uses `#[thread_local]` instead of pthread key.
2. when using emutls, decorate symbol names to find thread local symbol correctly.
3. change `-Zforce-emulated-tls` to `-Ztls-model=emulated` to explicitly specify whether to generate emutls.

r? `@Amanieu`
bors added a commit to rust-lang/miri that referenced this pull request Dec 10, 2023
@bors
Auto merge of #117873 - quininer:android-emutls, r=Amanieu
1ce0cb4 
Add emulated TLS support

This is a reopen of rust-lang/rust#96317 . many android devices still only use 128 pthread keys, so using emutls can be helpful.

Currently LLVM uses emutls by default for some targets (such as android, openbsd), but rust does not use it, because `has_thread_local` is false.

This commit has some changes to allow users to enable emutls:

1. add `-Zhas-thread-local` flag to specify that std uses `#[thread_local]` instead of pthread key.
2. when using emutls, decorate symbol names to find thread local symbol correctly.
3. change `-Zforce-emulated-tls` to `-Ztls-model=emulated` to explicitly specify whether to generate emutls.

r? `@Amanieu`
GuillaumeGomez pushed a commit to GuillaumeGomez/rustc_codegen_gcc that referenced this pull request Feb 15, 2024
@bors
Auto merge of #117873 - quininer:android-emutls, r=Amanieu
c578f43 
Add emulated TLS support

This is a reopen of rust-lang/rust#96317 . many android devices still only use 128 pthread keys, so using emutls can be helpful.

Currently LLVM uses emutls by default for some targets (such as android, openbsd), but rust does not use it, because `has_thread_local` is false.

This commit has some changes to allow users to enable emutls:

1. add `-Zhas-thread-local` flag to specify that std uses `#[thread_local]` instead of pthread key.
2. when using emutls, decorate symbol names to find thread local symbol correctly.
3. change `-Zforce-emulated-tls` to `-Ztls-model=emulated` to explicitly specify whether to generate emutls.

r? `@Amanieu`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
@bors
Auto merge of #117873 - quininer:android-emutls, r=Amanieu
3f72972 
Add emulated TLS support

This is a reopen of rust-lang/rust#96317 . many android devices still only use 128 pthread keys, so using emutls can be helpful.

Currently LLVM uses emutls by default for some targets (such as android, openbsd), but rust does not use it, because `has_thread_local` is false.

This commit has some changes to allow users to enable emutls:

1. add `-Zhas-thread-local` flag to specify that std uses `#[thread_local]` instead of pthread key.
2. when using emutls, decorate symbol names to find thread local symbol correctly.
3. change `-Zforce-emulated-tls` to `-Ztls-model=emulated` to explicitly specify whether to generate emutls.

r? `@Amanieu`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
@bors
Auto merge of #117873 - quininer:android-emutls, r=Amanieu
1f9797c 
Add emulated TLS support

This is a reopen of rust-lang/rust#96317 . many android devices still only use 128 pthread keys, so using emutls can be helpful.

Currently LLVM uses emutls by default for some targets (such as android, openbsd), but rust does not use it, because `has_thread_local` is false.

This commit has some changes to allow users to enable emutls:

1. add `-Zhas-thread-local` flag to specify that std uses `#[thread_local]` instead of pthread key.
2. when using emutls, decorate symbol names to find thread local symbol correctly.
3. change `-Zforce-emulated-tls` to `-Ztls-model=emulated` to explicitly specify whether to generate emutls.

r? `@Amanieu`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Amanieu Amanieu

Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@name1e5s @rust-highfive @Mark-Simulacrum @Amanieu @bors @GuillaumeGomez @JohnCSimon @rustbot