std: Add a variant of thread locals with const init#83416

alexcrichton · 2021-03-23T18:10:35Z

This commit adds a variant of the thread_local! macro as a new
thread_local_const_init! macro which requires that the initialization
expression is constant (e.g. could be stuck into a const if so
desired). This form of thread local allows for a more efficient
implementation of LocalKey::with both if the value has a destructor
and if it doesn't. If the value doesn't have a destructor then with
should desugar to exactly as-if you use #[thread_local] given
sufficient inlining.

The purpose of this new form of thread locals is to precisely be
equivalent to #[thread_local] on platforms where possible for values
which fit the bill (those without destructors). This should help close
the gap in performance between thread_local!, which is safe, relative
to #[thread_local], which is not easy to use in a portable fashion.

rust-highfive · 2021-03-23T18:10:38Z

r? @sfackler

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

sfackler · 2021-03-23T19:44:52Z

library/std/src/thread/local.rs

Does wasm32 declare support for target_thread_local?

It does, yes, beacuse it's supported with atomics enabled. Last I checked (a year or two ago) it just didn't behave well in LLVM if atomics was disabled.

Probably best to file an issue tracking that.

sfackler · 2021-03-23T19:46:43Z

library/std/src/thread/local.rs

Why is this infinitely recursive?

Oh this is actually recursing into a free-function so this shouldn't be infinitely recursive. The only purpose of this is for the macro-generated code to have a public function to call into, but I can document this better too.

Oh yeah missed this was an associated function.

sfackler · 2021-03-23T19:58:12Z

Is this intended for internal-only use, or do you see us stabilizing this at some point? It seems pretty unfortunate to have two macros like this in a stable public API.

alexcrichton · 2021-03-23T20:22:04Z

My intent would be to have this stabilized at some point. This gap in Rust's thread-local support has come up from time to time and I'd hope that this would get us much closer to closing the gap on performance.

I agree though that the API isn't great. Ideally I'd like to do something like test whether the input expression to thread_local! is const and change the expansion of the macro based on that, but I doubt we're likely to get such a feature any time soon. Another alternative would be alternative syntax in the original macro, such as:

thread_local!(const FOO: u32 = 3);
thread_local!(static FOO: u32 = const 3);

(etc)

One part I wasn't sure about though is that if an alternative syntax is chosen I'm not sure how to make just the new syntax unstable while keeping the current syntax stable.

sfackler · 2021-04-02T22:46:23Z

Oops, missed that you replied!

One option is the const { ... } blocks that have been (will be?) added to force const evaluation of an expression:

thread_local! {
    static FOO: u32 = const { 3 };
}

alexcrichton · 2021-04-05T14:57:15Z

Oh nice! I've updated to use that syntax (and also discovered a way to make only some arms of a macro unstable while the others are stable)

sfackler · 2021-04-12T17:31:52Z

library/std/src/thread/local.rs

Nit: this could use drop_in_place?

Oh this specifically avoids that since it's caused issues in the past with macOS and TLS I think, but I'll add some comments to this effect.

Nope turns out I was misremembering, updated to use drop_in_place

sfackler · 2021-04-12T17:33:54Z

r=me with a tracking issue and that one nit addressed.

alexcrichton · 2021-04-15T15:56:21Z

I've filed #84223 to track this feature and #84224 for the wasm issue with target_thread_local being set when it probably shouldn't.

@bors: r=sfackler

bors · 2021-04-15T15:56:23Z

📌 Commit 82cef56b15139a03092ef5d6636dbfa3f7e722b4 has been approved by sfackler

alexcrichton · 2021-04-15T16:20:06Z

@bors: r-

This commit adds a variant of the `thread_local!` macro as a new `thread_local_const_init!` macro which requires that the initialization expression is constant (e.g. could be stuck into a `const` if so desired). This form of thread local allows for a more efficient implementation of `LocalKey::with` both if the value has a destructor and if it doesn't. If the value doesn't have a destructor then `with` should desugar to exactly as-if you use `#[thread_local]` given sufficient inlining. The purpose of this new form of thread locals is to precisely be equivalent to `#[thread_local]` on platforms where possible for values which fit the bill (those without destructors). This should help close the gap in performance between `thread_local!`, which is safe, relative to `#[thread_local]`, which is not easy to use in a portable fashion.

alexcrichton · 2021-04-16T16:21:52Z

@bors: r=sfackler

bors · 2021-04-16T16:21:54Z

📌 Commit 82cef56b15139a03092ef5d6636dbfa3f7e722b4 has been approved by sfackler

alexcrichton · 2021-04-16T16:22:14Z

@bors: r=sfackler

bors · 2021-04-16T16:22:16Z

📌 Commit c6eea22 has been approved by sfackler

bors · 2021-04-16T16:36:50Z

⌛ Testing commit c6eea22 with merge 0cc00c4...

bors · 2021-04-16T19:25:18Z

☀️ Test successful - checks-actions
Approved by: sfackler
Pushing 0cc00c4 to master...

library/std/src/thread/local.rs

…=alexcrichton fix aliasing violations in thread_local_const_init Fixes rust-lang#83416 (comment) r? `@alexcrichton` `@sfackler`

jethrogb · 2021-04-21T07:49:43Z

library/std/src/thread/local/tests.rs

+    .unwrap();
+
+    unsafe {
+        HITS = 0;


This assumes that the TLS destructors have run by the time join() returns. This is currently not the case for the x86_64-fortanix-unknown-sgx target.

I'm a bit torn on whether this is the bug in the x86_64-fortanix-unknown-sgx implementation or this assumption is invalid.

TLS destructors need to run on the thread that is in the process of exiting. I think join() shouldn't return until thread is completely done exiting and thus would have finished running the TLS destructors.

I agree, having a thread do anything after join returned is highly suspicious.

…r=dtolnay Ensure TLS destructors run before thread joins in SGX The excellent test is from `@jethrogb` For context see: rust-lang#83416 (comment)

…r=dtolnay Ensure TLS destructors run before thread joins in SGX The excellent test is from ``@jethrogb`` For context see: rust-lang#83416 (comment)

…r=dtolnay Ensure TLS destructors run before thread joins in SGX The excellent test is from ```@jethrogb``` For context see: rust-lang#83416 (comment)

matklad · 2021-05-03T19:04:53Z

Oh wow, this is awesome, thanks alexcrichton!

…r=dtolnay Ensure TLS destructors run before thread joins in SGX The excellent test is from `@jethrogb` For context see: rust-lang#83416 (comment)

…r=dtolnay Ensure TLS destructors run before thread joins in SGX The excellent test is from ``@jethrogb`` For context see: rust-lang#83416 (comment)

…r=jethrogb Ensure TLS destructors run before thread joins in SGX The excellent test is from `@jethrogb` For context see: rust-lang#83416 (comment)

…r=jethrogb Ensure TLS destructors run before thread joins in SGX The excellent test is from ``@jethrogb`` For context see: rust-lang#83416 (comment)

…r=jethrogb Ensure TLS destructors run before thread joins in SGX The excellent test is from ```@jethrogb``` For context see: rust-lang#83416 (comment)

Amanieu · 2021-09-08T22:17:48Z

library/std/src/thread/local.rs

+                // just get going.
+                if !$crate::mem::needs_drop::<$t>() {
+                    #[thread_local]
+                    static mut VAL: $t = $init;


The mut here seems unnecessary.

…d, r=thomcc Document `const {}` syntax for `std::thread_local`. It exists and is pretty cool. More people should use it. It was added in rust-lang#83416 and stabilized in rust-lang#91355 with the tracking issue rust-lang#84223.

rust-highfive assigned sfackler Mar 23, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 23, 2021

This comment has been minimized.

Sign in to view

alexcrichton force-pushed the const-thread-local branch from 85ca7d3 to ab4622f Compare March 23, 2021 18:43

sfackler reviewed Mar 23, 2021

View reviewed changes

alexcrichton force-pushed the const-thread-local branch from ab4622f to 0b86126 Compare April 5, 2021 14:56

alexcrichton force-pushed the const-thread-local branch from 0b86126 to 6555364 Compare April 5, 2021 14:58

sfackler reviewed Apr 12, 2021

View reviewed changes

This was referenced Apr 15, 2021

Tracking Issue for const-initialized thread locals #84223

Closed

#[cfg(target_thread_local)] is true for WebAssembly targets without atomics enabled #84224

Open

alexcrichton force-pushed the const-thread-local branch from 6555364 to 82cef56 Compare April 15, 2021 15:55

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 15, 2021

This comment has been minimized.

Sign in to view

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 15, 2021

bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 16, 2021

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 16, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 16, 2021

bors merged commit 0cc00c4 into rust-lang:master Apr 16, 2021

rustbot added this to the 1.53.0 milestone Apr 16, 2021

RalfJung reviewed Apr 18, 2021

View reviewed changes

library/std/src/thread/local.rs Show resolved Hide resolved

RalfJung mentioned this pull request Apr 18, 2021

fix aliasing violations in thread_local_const_init #84291

Merged

jethrogb reviewed Apr 21, 2021

View reviewed changes

mzohreva mentioned this pull request Apr 21, 2021

Ensure TLS destructors run before thread joins in SGX #84409

Merged

alexcrichton mentioned this pull request May 3, 2021

Avoid initialization checks when accessing thread locals #44025

Closed

alexcrichton deleted the const-thread-local branch September 2, 2021 16:33

Amanieu reviewed Sep 8, 2021

View reviewed changes

arshiyasolei mentioned this pull request Nov 9, 2022

enhancement(core): const init thread locals vectordotdev/vector#15158

Merged

AnthonyMikh mentioned this pull request Apr 3, 2023

Document const-initialization for thread_local! macro #109908

Closed

Noratrieb mentioned this pull request Apr 20, 2023

Document const {} syntax for std::thread_local. #110620

Merged

Uh oh!

Conversation

alexcrichton commented Mar 23, 2021

Uh oh!

rust-highfive commented Mar 23, 2021

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfackler commented Mar 23, 2021

Uh oh!

alexcrichton commented Mar 23, 2021

Uh oh!

sfackler commented Apr 2, 2021

Uh oh!

alexcrichton commented Apr 5, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfackler commented Apr 12, 2021

Uh oh!

alexcrichton commented Apr 15, 2021

Uh oh!

bors commented Apr 15, 2021

Uh oh!

This comment has been minimized.

alexcrichton commented Apr 15, 2021

Uh oh!

alexcrichton commented Apr 16, 2021

Uh oh!

bors commented Apr 16, 2021

Uh oh!

alexcrichton commented Apr 16, 2021

Uh oh!

bors commented Apr 16, 2021

Uh oh!

bors commented Apr 16, 2021

Uh oh!

bors commented Apr 16, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matklad commented May 3, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants