Optimize LazyLock size

[joboet]

The initialization function was unnecessarily stored separately from the data to be initialized. Since both cannot exist at the same time, a union can be used, with the Once acting as discriminant. This unfortunately requires some extra methods on Once so that Drop can be implemented correctly and efficiently.

@rustbot label +T-libs +A-atomic

[rustbot]

r? @joshtriplett

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

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 @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. 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

[Kobzol]

Since this optimizes the size, it would be nice to add a static_assert_size!(LazyLock, XXX); somewhere.

[joboet]

Since this optimizes the size, it would be nice to add a static_assert_size!(LazyLock, XXX); somewhere.

I'm a bit hesitant to do so because the size is very platform and compiler specific. Also, I'm not really convinced that a test is necessary here. If required, however, would something like this suffice?

/// Test that the total size is less than the size required to store both the closure
/// and the value at the same time.
#[test]
fn test_size() {
    const SIZE: usize = 1024;

    type InitFn = impl FnOnce() -> [u8; SIZE];

    fn init_fn(data: [u8; SIZE]) -> InitFn {
        move || data
    }

    assert!(size_of::<LazyLock<[u8; SIZE], InitFn>>() < size_of::<Once>() + SIZE + size_of::<InitFn>());
}

[Kobzol]

The size asserts are usually checked only for x64:

rust/compiler/rustc_ast/src/tokenstream.rs

Line 644 in c62665e

#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]

. But it's not needed for every structure of course.

[joboet]

The size asserts are usually checked only for x64:

rust/compiler/rustc_ast/src/tokenstream.rs

Line 644 in c62665e

#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]

. But it's not needed for every structure of course.

The problem is that even on systems with the same pointer size, Once has a different size and alignment on Linux (4 bytes) that on e.g. macOS (8 bytes). So the test would need to be limited to one system with one pointer width, which doesn't seem very useful. IMHO the change in implementation alone is enough to guarantee the improvements.

[m-ou-se]

Nit: With a #[derive(Debug)] struct Uninit;, a fully initialized LazyLock<Uninit> would also produce "LazyLock(Uninit)". Maybe it's better to show a nested Option (LazyLock(Some(..))), or to use LazyLock(<uninitialized>) for the uninitialized case.

[m-ou-se]

Oh I see we also already format an empty OnceCell as OnceCell(Uninit). So no need to change that in this PR.

[m-ou-se]

@bors r+

[bors]

📌 Commit 6520488 has been approved by m-ou-se

It is now in the queue for this repository.

[bors]

⌛ Testing commit 6520488 with merge 3701bdc...

[bors]

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing 3701bdc to master...

[rust-timer]

Finished benchmarking commit (3701bdc): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.7%, 0.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.4%	[-1.4%, -1.4%]	1
All ❌✅ (primary)	0.7%	[0.7%, 0.7%]	1

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

[SimonSapin]

This unfortunately requires some extra methods on Once so that Drop can be implemented correctly and efficiently.

If this new Once method is useful to LazyLock, could it be useful to other similar types defined outside of std? Should there be a tracking issue to consider making it a public API?

[joboet]

This unfortunately requires some extra methods on Once so that Drop can be implemented correctly and efficiently.

If this new Once method is useful to LazyLock, could it be useful to other similar types defined outside of std? Should there be a tracking issue to consider making it a public API?

I don't anticipate a need for it, considering that LazyLock should hopefully be part of stable std at some point. It is currently an internal API only, so making it public would need an ACP and a new PR.