-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Tracking Issue for once_cell
#74465
Comments
Let's cross-out the "should get be blocking?" concern. I decided against this for once_cell, for the following reasons:
|
Added two more open questions from the RFC. |
This reverts commit fe63905.
I've added a summary of proposed API to the issue description. I wonder if makes sense for @rust-lang/libs to do a sort of "API review" here: this is a pretty big chunk of API, and we tried to avoid bike shedding on the RFC. |
Here's an interesting use-case for non-blocking subset of OnceCell -- building cyclic data structures: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=4eceeefc224cdcc719962a9a0e1f72fc |
I strongly expect a method called |
Yeah, to be clear, there's a consensus that |
I don't think I'd expect something that's named AtomicLazy/AtomicOnceCell to do the same. And that's something that already exists as another valid strategy for certain Lazy/OnceCell-like types: Instead of blocking all but one thread when multiple threads encounter an 'empty' cell, it wouldn't block but run the initialization function on each of these threads. The first thread to finish atomically stores its initialized value in the cell, and the others simply The rust/library/std/src/sys/windows/compat.rs Lines 65 to 67 in a835b48
And here: rust/library/std/src/sys/windows/mutex.rs Lines 119 to 133 in a835b48
And another example in So, since this Lazy/OnceCell implementation does block (such that the initialization function can be |
I've added non-blocking flavors of the primitives to the once_cell crate: https://docs.rs/once_cell/1.5.1/once_cell/race/index.html. They are restricted (can be provided only for atomic types), but are compatible with no_std. It seems to me that "first one wins" is a better semantics if you can't block, so I am going to resolve
I've ticked this question's box. |
It's a bit of a shame that |
This seems like a very major restriction, which rules out most use cases of
I agree that spinlocks have their problems, but they're still better than using This could be implemented using a second generic argument on the |
Some thoughts about These types have both Maybe if the |
Unresolved question: method naming Currently, we have
1. Pro: Status Quo, name specific to I've though more about this, and I think I actually like 3 most. It's Con seems like a Pro to me. In the typical use-case, you only use impl Spam {
fn get_eggs(&self) -> &Eggs {
self.eggs.get_with(|| Eggs::cook())
}
} So, the closure is sort-of always called, it's just cached. Not sure if I my explanation makes sense, but I do feel that this is different from, eg, |
@phil-opp: I think it is rather certain that, even if std provides a subset of OnceCell for no_std, it will be non-blocking subset ( It certainly is possible to use spinlocks, or make sync::OnceCell parametric (compile-time or run-time) over blocking primitives. I am pretty sure that should be left for crates.io crate though. I feel one important criterion for inclusion in std is "design space has a solution with a single canonical API". OnceCell API seem canonical. If we add paramters, the design space inflates. Even if some solution would be better, it won't be obviously canonical, and would be better left to crates.io. |
@m-ou-se yeah, totally agree that this is a hack and feels like a hack. It works well enough in practice, but there's one gotcha: specifying type for a local lazy does not work: let x = 92;
let works1: = Lazy::new(|| x.to_string());
let broken: Lazy<String> = Lazy::new(|| x.to_string());
let works2: Lazy<String, _> = Lazy::new(|| x.to_string()); The One easy way out here is to stabilize only fn global_state() -> &'static GlobalState {
static INSTANCE: SyncOnceCell<GlobalState> = SyncOnceCell::new();
INSTANCE.get_or_init(GlobalState::default)
} doesn't feel like a deal breaker.I'd prefer that to pulling a 3rd party dep (lazy_staic or once_cell). That said, I think static GLOBAL_STATE: Lazy<GlobalState, _> = Lazy::new(GlobalState::default); I don't see a lot of practical problems with static GLOBAL_STATE: Lazy<GlobalState> = Lazy::new(GlobalState::default); working as well. |
I think 1 is the most appropriate. The
This doesn't seem very intuitive to me and isn't always true when there are multiple points of initialization. For example, consider: impl Spam {
fn get_eggs(&self, cooked: bool) -> &Eggs {
if cooked {
self.eggs.set(Eggs::cook());
}
self.eggs.get_with(|| Eggs::raw())
}
} In this case, the closure may not run and in fact a different value has been cached. I think |
Something I've recently got bitten by is the need to manage which memory allocator a memory uses. I've been workign wit ha design that has a different global memory allocator when running threads or coroutines (so restricting a coroutine to a maximum amount of memory). This could be thought of as a bit of a hack; one of the long-term design decisions of early Rust that still bites is not making the memory allocator type explicit in the standard collections. With a lazy, the challenge becomes ensuring that they're all allocated using the same memory allocator. |
Can we have a |
|
Can't you say the same about |
Because not all accessors are You can read this code snippet and see if there is a better solution than providing a // batches: OnceCell<Vec<RecordBatch>>,
pub fn mut_batches(&mut self) -> IterMut<'_, RecordBatch> {
self.batches.get_or_init(|| load_batches(&self.buf));
// SAFETY - init above
unsafe { self.batches.get_mut().unwrap_unchecked() }.iter_mut()
}
pub fn batches(&self) -> Iter<'_, RecordBatch> {
self.batches.get_or_init(|| load_batches(&self.buf)).iter()
} |
But you're right that without concurrent calls an |
No. I need to guard shared non-mut access to Otherwise, pub fn batches(&self) -> Iter<'_, RecordBatch> {
self.batches
.get_or_insert_with(|| load_batches(&self.buf))
.iter()
} failed to compile: OnceCell impl |
See also rust-lang#74465 (comment) Signed-off-by: tison <wander4096@gmail.com>
cargo-test-sbf (governance/addin-mock/program, governance/program): error[E0658]: use of unstable library feature 'once_cell' --> src/abi_example.rs:559:36 | 559 | impl<T: AbiExample> AbiExample for std::sync::OnceLock<T> { | ^^^^^^^^^^^^^^^^^^^^^^ | = note: see issue #74465 <rust-lang/rust#74465> for more information = help: add `#![feature(once_cell)]` to the crate attributes to enable
* Define generic AbiExample for OnceLock * Guard with cfg... cargo-test-sbf (governance/addin-mock/program, governance/program): error[E0658]: use of unstable library feature 'once_cell' --> src/abi_example.rs:559:36 | 559 | impl<T: AbiExample> AbiExample for std::sync::OnceLock<T> { | ^^^^^^^^^^^^^^^^^^^^^^ | = note: see issue #74465 <rust-lang/rust#74465> for more information = help: add `#![feature(once_cell)]` to the crate attributes to enable
* Define generic AbiExample for OnceLock * Guard with cfg... cargo-test-sbf (governance/addin-mock/program, governance/program): error[E0658]: use of unstable library feature 'once_cell' --> src/abi_example.rs:559:36 | 559 | impl<T: AbiExample> AbiExample for std::sync::OnceLock<T> { | ^^^^^^^^^^^^^^^^^^^^^^ | = note: see issue #74465 <rust-lang/rust#74465> for more information = help: add `#![feature(once_cell)]` to the crate attributes to enable
Due to an indirect dependency which fails with the following error on 1.65.0 --- error[E0658]: use of unstable library feature 'once_cell' --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ignore-0.4.21/src/gitignore.rs:596:9 | 596 | use std::sync::OnceLock; | ^^^^^^^^^^^^^^^^^^^ | = note: see issue #74465 <rust-lang/rust#74465> for more information
See also rust-lang#74465 (comment) Signed-off-by: tison <wander4096@gmail.com>
See also rust-lang#74465 (comment) Signed-off-by: tison <wander4096@gmail.com>
See also rust-lang#74465 (comment) Signed-off-by: tison <wander4096@gmail.com>
See also rust-lang#74465 (comment) Signed-off-by: tison <wander4096@gmail.com>
impl get_mut_or_init and get_mut_or_try_init for OnceCell and OnceLock See also rust-lang#74465 (comment) I'm trying to understand the process for such proposal. And I'll appreciate it if anyone can guide me the next step for consensus or adding tests.
impl get_mut_or_init and get_mut_or_try_init for OnceCell and OnceLock See also rust-lang#74465 (comment) I'm trying to understand the process for such proposal. And I'll appreciate it if anyone can guide me the next step for consensus or adding tests.
impl get_mut_or_init and get_mut_or_try_init for OnceCell and OnceLock See also rust-lang#74465 (comment) I'm trying to understand the process for such proposal. And I'll appreciate it if anyone can guide me the next step for consensus or adding tests.
impl get_mut_or_init and get_mut_or_try_init for OnceCell and OnceLock See also rust-lang#74465 (comment) I'm trying to understand the process for such proposal. And I'll appreciate it if anyone can guide me the next step for consensus or adding tests.
Rollup merge of rust-lang#114788 - tisonkun:get_mut_or_init, r=dtolnay impl get_mut_or_init and get_mut_or_try_init for OnceCell and OnceLock See also rust-lang#74465 (comment) I'm trying to understand the process for such proposal. And I'll appreciate it if anyone can guide me the next step for consensus or adding tests.
Are there any plans for making |
@sdroege You can do `OnceLock::get().unwrap_unchecked(). |
Thanks, the atomic op and branch in |
This is a tracking issue for the RFC "standard lazy types" (rust-lang/rfcs#2788).
The feature gate for the issue is
#![feature(once_cell)]
.Unstable API
Steps
once_cell
#105587 (comment)once_cell
#105587Unresolved Questions
Inlined from #72414:
Sync
prefix likeSyncLazy
for now, but have a personal preference forAtomic
likeAtomicLazy
. Resolved in: Tracking Issue foronce_cell
#74465 (comment). Surprisingly, after more than a year of deliberation we actually found a better name.std::sync
types that we might want to just avoid upfront forstd::lazy
, especially if that would align with a futurestd::mutex
that doesn't poison. Personally, if we're adding these types tostd::lazy
instead ofstd::sync
, I'd be on-board with not worrying about poisoning instd::lazy
, and potentially deprecatingstd::sync::Once
andlazy_static
in favour ofstd::lazy
down the track if it's possible, rather than attempting to replicate their behavior. cc @Amanieu @sfackler.SyncOnceCell::get
blocking. There doesn't seem to be consensus in the linked PR on whether or not that's strictly better than the non-blocking variant. (resolved in Tracking Issue foronce_cell
#74465 (comment)).Release/Acquire
, but it could also use the elusive Consume ordering. Should we spec that we guaranteeRelease/Acquire
? (resolved as yes: consume ordering is not defined enough to merit inclusion into std)SyncOnceCell
in no_std. I think there's consensus that we don't want to include "blocking" parts of API, but it's unclear if non-blocking subset (get+set) would be useful. (resolved in Tracking Issue foronce_cell
#74465 (comment)).get_or[_try]_init
the best name? (resolved as yes in Update once_cell 'get_or_init' to 'get_or_init_with' #107184)Implementation history
UnwindSafe
boundsThe text was updated successfully, but these errors were encountered: