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

Replace lazy_static with once_cell #3187

Merged
merged 3 commits into from
Dec 4, 2020
Merged

Replace lazy_static with once_cell #3187

merged 3 commits into from
Dec 4, 2020

Conversation

Razican
Copy link
Contributor

@Razican Razican commented Nov 27, 2020

Motivation

once_cell is a macro-free alternative to lazy_static. Its Lazy type has the same API as lazy_static, but it will not use any "macro magic" and it might be more clear to read by non-experienced Rust developers.

This solution is being proposed to be implemented at the Rust standard library level with an RFC. This could be an opportunity to migrate to the new solution and to check that everything is working correctly in Tokio.

Furthermore, as per this comment, it seems that due to a possible compiler bug, using once_cell is noticeably faster than lazy_static in certain hot get operations.

Solution

This PR replaces all mentions to lazy_static by once_cell using the built-in Lazy type.

Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of this. lazy_static has some other (potential) issues (rust-lang-nursery/lazy-static.rs#150, rust-lang/futures-rs#1485 (comment)).

@taiki-e taiki-e added A-tokio Area: The main tokio crate C-maintenance Category: PRs that clean code up or issues documenting cleanup. and removed C-maintenance Category: PRs that clean code up or issues documenting cleanup. labels Nov 27, 2020
@Darksonn Darksonn added M-process Module: tokio/process M-signal Module: tokio/signal labels Nov 28, 2020
@carllerche
Copy link
Member

This PR is fine as a first step.

However, I am noticing that once_cell has support for parking_lot as an optional feature. Ideally, we could enable that feature when we enable that feature, but cargo does not support this.

Instead, we may consider vendoring once_cell to work around this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-process Module: tokio/process M-signal Module: tokio/signal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants