Tracking Issue for const-initialized thread locals

[alexcrichton]

Feature gate: #![feature(thread_local_const_init)]

This is a tracking issue for the const-initialization of thread locals in the standard library added in #83416.

Public API

use std::cell::Cell;
std::thread_local!(static KEY: Cell<u32> = const { Cell::new(0) });

// ... same APIs as before with thread locals

The API here is intended to be exclusively within the syntax of the preexisting thread_local! macro. The initialization expression for thread local keys can now be surrounded with const { ... } to indicate that the value should be const-evaluated and initialized.

Note that this also supports the multi-definition form:

use std::cell::Cell;
std::thread_local! {
    static KEY1: Cell<u32> = const { Cell::new(0) };
    static KEY2: Cell<u32> = const { Cell::new(0) };
}

Steps / History

• Implementation: std: Add a variant of thread locals with const init #83416
• Final commenting period (FCP)
• Stabilization PR

Unresolved Questions

• Syntax bikeshed - is the use of what is likely to be a future-feature, const blocks, ok here? Are there other options for syntax?

[alexcrichton]

I've requested this be tagged I-nominated for discussion for stabilization, but I figured I'd want to paste some background here as well. My personal main motivation for this is optimizing the pieces of Wasmtime where a host calls into a WebAssembly module. Part of that is frobbing some TLS values as we enter into wasm, and this frobbing is actually marked #[inline(never)] for the poor cross-crate codegen that TLS has otherwise. This outlining plus the extra checks for whether or not TLS is initialized is what I'm trying to optimize.

In benchmarks locally the call overhead of host->wasm decreases from 20ns to 18ns (10% reduction) with the usage of const-initialized-thread-locals (which are also #[inline]'d since they have good codegen). This isn't really gonna make or break anyone's day since it's a pretty small difference, but nevertheless I'd love to get it out of the way so I don't have to worry about it again.

Another data point I see linked here is that rustc is using const-initialized thread locals for some moderate gains across the board.

There was some brief discussion on Zulip about the syntax when I originally asked to stabilize. One thing I'll clarify is that this doesn't literally use const { .. } blocks in the expanded code, the macro is simply recognizing it as input syntax. I believe one gotcha of this is that if you have $e:expr in a macro and in the future $e is something like const { 1 } I don't think that it will use the const-initialized version of thread_local! were you to do something like thread_local!(static X: i32 = $e) since I don't think that rustc splits apart tokens to match sub-patterns in a macro. This will, however, work for literal declarations like thread_local!(static X: i32 = const { 1 });

Overall the biggest thing to decide here I think is the syntax. One alternative is a whole new macro, but I think that it'd be ideal if we could keep it in one macro to not have to worry about multiple. Otherwise though I'm happy to implement whatever syntax.

[m-ou-se]

Discussed in the library api team meeting. We'd like @rust-lang/lang to sign off on this.

[nikomatsakis]

This seems like a fine thing to stabilize; however, just to be clear, is this feature any use without const blocks? I guess that one can still stabilize to other constants (e.g., simple integer values)?

[alexcrichton]

This doesn't actually use const { .. } blocks at the compiler level, it's basically just surface syntax to the macro itself. In that sense this should still be useful even if Rust never gets const { .. } blocks because it would still trigger the specialized implementation for this macro where the initialization expression is a constant known at compile time

[cramertj]

It seems a little funky to me to stabilize a syntax that, at a surface level, matches that of a pre-existing unstable feature (const blocks). Even though it may not require const blocks under the hood, in the long-term I think users would expect that the set of things allowed inside thread_local!(static X: Y = const { <code here> }; would match the rules for what could be present in static X: Y = const { ... };.

That is, if we decided that some surface syntax or semantics should be different inside const blocks, I'd also want it to be different in these not-const-blocks. Are we confident that we'll be able to provide matching semantics in each of these contexts in the future? That is, that the not-const-block in thread_local! doesn't allow anything that a const block wouldn't allow?

[nikomatsakis]

Oh, wait, maybe I misunderstood. I didn't realize that const { ... } was part of the macro, I expected it to accept "any expression". That said, I'll have to review, but I would expect that indeed it should accept the same set of code?

[alexcrichton]

For reference, the actual macro is (trimmed down a bit):

macro_rules! thread_local {
    ($(#[$attr:meta])* $vis:vis static $name:ident: $t:ty = const { $init:expr }) => (
        // ...
    );

    ($(#[$attr:meta])* $vis:vis static $name:ident: $t:ty = $init:expr) => (
        // ...
    );

    // ...
}

The actual expanded code ends up doing:

                    #[thread_local]
                    static mut VAL: $t = $init;

I believe what @cramertj is describing is indeed a risk of using the const { ... } syntax in the macro without literally using it in the expanded code. If for whatever reason the code allowed inside of const { ... } differed from that in { ... } then this macro expansion would not reflect that becaue the expansion doesn't actually use const { .. }.

Personally I'm fine implementing some other syntax if there's too many worries about using the const { ... } syntax. What I would probably suggest is an alternative macro, thread_local_const!, or something like that (although I realize there are downsides to this as well and would welcome other thoughts on alternative syntaxes)

[nikomatsakis]

@rfcbot fcp merge

We discussed this in the @rust-lang/lang meeting today. Consensus was that we should move forward with stabilization here, so I am beginning an FCP.

Here are our notes:

• What is the change?
  • Macro looks for const { expr } and changes the code that gets generated
    • currently this gets generated to static FOO: ... = <expr>
• Why this change?
  • Codegen improvement: the initial value of a thread-local is constant and not re-evaluated per thread
• Expected outcome?
  • We stabilize the macro and eventually move it to use inline constants
• What is the risk?
  • The macro currently accepts const { ... } syntax but desugars to a static
  • If the set of expressions accepted as initial value for a static diverged from set that can appear in const { ... } this would be weird, but we consider this unlikely.
  • Moreover, if that were to occur in the future before const is stabilied, we could add const { ... } into the desugaring and use "allow internal unstable",

[rfcbot]

Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @cramertj
• @dtolnay
• @joshtriplett
• @m-ou-se
• @nikomatsakis
• @pnkfelix
• @scottmcm
• @yaahc

Concerns:

• LocalKey type resolved by Tracking Issue for const-initialized thread locals #84223 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[cramertj]

Moreover, if that were to occur in the future before const is stabilied, we could add const { ... } into the desugaring and use "allow internal unstable",

I don't understand how this would work. If the set of things allowed in const is limited, it'd be a breaking change to move this macro over to use const after this macro has already been available on stable. I'm happy to land as-is if we think the risk of these sorts of additional limitations is low, though.

[yaahc]

* If the set of expressions accepted as initial value for a static diverged from set that can appear in `const { ... }` this would be weird, but we consider this unlikely.

If this happened I believe we could fix the inconsistency at an edition boundary the same way we did for panic!, so I don't think this is a particularly large risk.

[m-ou-se]

@rfcbot concern LocalKey type

A variable defined with thread_local!{} is of the type LocalKey<T>. I'm wondering if we should use a different type for const-initialized thread locals.

Right now, reusing the same type works fine, as it just a wrapped function pointer. But I can imagine that a thread local key without lazy initializer might have a different implementation or even api than the one with. (E.g. THREAD_LOCAL.is_initialized() or THREAD_LOCAL.initialize_with(|| ..).)

[BurntSushi]

@rust-lang/lang

If the set of expressions accepted as initial value for a static diverged from set that can appear in const { ... } this would be weird, but we consider this unlikely.

Moreover, if that were to occur in the future before const is stabilied, we could add const { ... } into the desugaring and use "allow internal unstable",

If this happened in a way where more things were accepted by a static than by const { ... }, wouldn't changing thread_local! to use const { ... } internally be a breaking change?

I wonder if we might indulge briefly in a thought experiment. If we knew that static items and const { ... } items diverged in some way, would we still use the const { ... } syntax here? If not, what would we use instead?

[alexcrichton]

With a concern about reusing LocalKey<T> for the underlying type for this, if it were to switch to something like ConstLocalKey<T> (or w/e name) then it would arguably warrant a different syntax as well along the lines of const_thread_local!(...) (or w/e name) which would help make the point of const { ... } blocks moot.

[nbdd0121]

I am concerned because this differs from the const block.

#![feature(inline_const)]
#![feature(thread_local_const_init)]

use std::cell::Cell;

static V: u32 = 1;

std::thread_local!{
	// This is accepted, because this isn't actually const.
    static KEY: Cell<u32> = const { Cell::new(V) };

	// This is not, because this is actually const, and const cannot refer to statics.
    static KEY2: Cell<u32> = (const { Cell::new(V) });
}

[alexcrichton]

To what extent are the rules for evaluating const { .. } different or the same as const X: ty = ...;? If they're the same then I think that the const-semantic worries could be pretty easily solved by changing the macro expansion to look like:

const INIT: $t = $init;
#[thread_local]
static mut VAL: $t = INIT;

There is still the more fundamental concern though about perhaps not wanting to use LocalKey at all.

[m-ou-se]

@rfcbot resolve LocalKey type

A thread_local_const_init or something like that would probably not only require the initialization expression to be const, but also forbid the type from needing Drop. If the type needs drop, it'll need to keep track of its initialization state anyway.

The only thing to note is that if we do ever add a .is_initialized() or anything similar to LocalKey, the current optimization might no longer work, since it'll need to track the 'initialization' state even when it doesn't need drop.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[veber-alex]

If this is a pure performance optimization, can the macro just "do the right thing" without any user facing syntax?
It will require some compiler magic but maybe it's worth it in this case?
This way also avoids any breaking changes in the future as what is optimized and what isn't is internal and all existing users will benefit without changing code or bumping MSRV.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[alexcrichton]

I've posted a stabilization PR for this at #91355

[Darksonn]

This has had a stabilization PR merged, but the todo list still lists the stabilization PR as missing.

[ahicks92]

I'm also not able to find docs on this other than that the macro now lists these patterns in the declaration: https://doc.rust-lang.org/std/macro.thread_local.html

(that is nothing says "and by the way..." but maybe I'm missing it?)

Nor can i find anything in the release notes or via Google. The only thing that seems to exist as to this being stable is that the PR merged a long time ago. What's the actual status? I haven't tried to use this yet on stable but presumably I can?

[Noratrieb]

@ahicks92 you're right, I opened #110620

[ahicks92]

Cool! thanks. TLS doesn't come up super often but there was at least one case not that long ago where we were nightly for the attribute version of this in perf sensitive code.