Skip to content

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