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.

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

Switch from lazy_static to once_cell #65

Closed
jplatte opened this issue Dec 30, 2020 · 7 comments
Closed

Switch from lazy_static to once_cell #65

jplatte opened this issue Dec 30, 2020 · 7 comments

Comments

@jplatte
Copy link

jplatte commented Dec 30, 2020

once_cell::sync::Lazy fulfills the same task as lazy_static but is less magic in that it doesn't require a macro. A lot of crates have switched already and std has adopted large parts of once_cells API surface as a Nightly feature (tracking issue). Would you be open to a PR that switches aHash over too?

@tkaitchuck
Copy link
Owner

I prefer once_cell. I actually started with it. However it pulls in a huge number of transitive dependencies (or at least it did when I last checked), and I would like to keep aHash as light as possible given how low level it is.

@jplatte
Copy link
Author

jplatte commented Jan 8, 2021

Just checked, once_cell has a single optional dependency (parking_lot). Same for version 1.0.1 (1.0.0 is yanked). Maybe a pre-1.0 version had more deps, but it's certainly not the case now 🙂

@tkaitchuck
Copy link
Owner

tkaitchuck commented Jan 8, 2021

I think the trouble is that aHash would need parking_lot. Per the doc:

To implement a sync flavor of OnceCell, this crates uses either a custom re-implementation of std::sync::Once or parking_lot::Mutex. This is controlled by the parking_lot feature, which is enabled by default. Performance is the same for both cases, but the parking_lot based OnceCell is smaller by up to 16 bytes.

Which is sort of a shame, because the whole reason for the need for the lock is to block to make sure the initialization code doesn't get run multiple times. However I don't actually care about that. If there were an "ApproximatelyOnceCell" with zero deps that would be ideal.

@jplatte
Copy link
Author

jplatte commented Jan 11, 2021

I don't exactly understand. Is it that you wouldn't want to use once_cell with a 16 byte overhead?

@tkaitchuck
Copy link
Owner

I don't care about the 16 bytes. I care about pulling in a dependency on parking_lot. If I can avoid having all those deps and get a working solution I'd be happy. I created matklad/once_cell#133 to track the issue. Either of matklad/once_cell#61 or matklad/once_cell#53 would also be acceptable.

@jplatte
Copy link
Author

jplatte commented Jan 11, 2021

Oh, so the issue is no_std support. That wasn't clear before. Thanks for following up with once_cell, I'm curious to see where that goes; it would be relevant for tracing too (tokio-rs/tracing#1165).

@tkaitchuck
Copy link
Owner

Ok. c828cc6
Will be included in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants