Migrate from lazy_static to once_cell #2313

james7132 · 2022-06-05T08:49:27Z

Tested on all platforms changed
Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
Created or updated an example program if it would help users understand this functionality
Updated feature matrix, if new features were added or implemented

Motivation

lazy_static!, while a declarative macro, is a macro nonetheless. It can add quite a bit of additional compilation time cost. once_cell::sync::Lazy does the same thing with generics, and can be used more flexibly (i.e. non-static lazily initialized values), and has been proposed to be added to std.

I'm trying to reduce the compile time and dependency tree complexity of a dependent project: bevy, which is using winit. lazy_static and once_cell are both in our dependency tree and both end up doing the same thing.

Solution

Migrate to once_cell.

I don't think any of the changes here are user visible beyond the dependency in the tree, and should work as-is on all of them, but I could be wrong.

src/platform_impl/linux/x11/monitor.rs

src/platform_impl/linux/x11/util/wm.rs

madsmtm

Sure, if it helps you. This is likely going to be closer to the final std API anyhow, and then the switch will be easier when we get there.

maroider

Mostly nitpicks from me. I don't quite get why you've grouped "external crate" imports with/above std imports. I was under the impression that std -> extern crate -> local crate is the "normal" way of grouping imports (or at least quite popular, as it's the only option besides "preserve" in rustfmt's configuration).

src/platform_impl/macos/app_state.rs

src/platform_impl/macos/window.rs

src/platform_impl/linux/x11/ime/input_method.rs

src/platform_impl/linux/mod.rs

kchibisov

I don't quite get why you've grouped "external crate" imports with/above std imports. I was under the impression that std -> extern crate -> local crate

I agree with @maroider . external crates should be separate from std. It just winit doesn't have formatting that way across the code base, but it doesn't mean that we should break it even more.

james7132 · 2022-06-08T11:29:46Z

Should have fixed the spurious moves as a result of cargo fmt --all, including some of the aforementioned broken ones.

Is there a way to add this via a rustfmt.toml? Seems like a trivial thing for any contributor to just run the command and undo all of these.

madsmtm · 2022-06-08T14:56:16Z

Is there a way to add this via a rustfmt.toml? Seems like a trivial thing for any contributor to just run the command and undo all of these.

There will be, it's just not stable yet.

maroider

Everything seems OK now. Like, @madsmtm, I'm of the opinion that we should move to the std API when that stabilizes, and this will make that switch easier.

MarijnS95 · 2022-06-09T09:24:22Z

There will be, it's just not stable yet.

Would it make sense to require this in the CI at least? Strangely enough the option works on a stable rustfmt when passed on the commandline, so it doesn't seem too harsh to request contributors to manually invoke something like cargo fmt -- --config imports_granularity=crate --config group_imports=stdexternalcrate.

Otherwise future PRs might accidentally break or not adhere to this "unwritten" convention, and puts the burden on reviewers to make sure things stay sane. Lots of files don't fully adhere to it yet either.

kchibisov · 2022-06-09T09:34:06Z

Or we require running rustfmt on nightly and run it on nightly on CI. That's how we deal with it in alacritty for example. On CI we install latest available rustfmt from nightly and run formatter.

Piggybacking on the [motivation in winit]: `lazy_static!` is a macro whereas `once_cell` achieves the same using generics. Its implementation has also been [proposed for inclusion in `std`], making it easier for us to switch to a standardized version if/when that happens. The author of that winit PR is making this change to many more crates, slowly turning the scales in favour of `once_cell` in our dependency tree too. Furthermore `lazy_static` hasn't published any updates for 3 years, and the new syntax is closer for dropping this wrapping completely when the necessary constructors become `const` (i.e. switching to `parking_lot` will give us a [`const fn new()` on `RwLock`]) or this feature lands in stable `std`. [motivation in winit]: rust-windowing/winit#2313 [proposed for inclusion in `std`]: rust-lang/rust#74465 [`const fn new()` on `RwLock`]: https://docs.rs/lock_api/latest/lock_api/struct.RwLock.html#method.new

MarijnS95 · 2022-06-10T17:49:37Z

@kchibisov I don't really mind which rustfmt to use, given that stable seems to work iff the config option is passed on the cmdline.

Would you do the honors and PR that rustfmt from alacritty here?

Piggybacking on the [motivation in winit]: `lazy_static!` is a macro whereas `once_cell` achieves the same using generics. Its implementation has also been [proposed for inclusion in `std`], making it easier to switch to a standardized version if/when that happens. The author of that winit PR is making this change to many more crates, slowly turning the scales in favour of `once_cell` in most dependency trees. Furthermore `lazy_static` hasn't published any updates for 3 years. See also [the `once_cell` F.A.Q.]. [motivation in winit]: rust-windowing/winit#2313 [proposed for inclusion in `std`]: rust-lang/rust#74465 [the `once_cell` F.A.Q.]: https://docs.rs/once_cell/latest/once_cell/#faq

Piggybacking on the [motivation in winit]: `lazy_static!` is a macro whereas `once_cell` achieves the same using generics. Its implementation has also been [proposed for inclusion in `std`], making it easier to switch to a standardized version if/when that happens. The author of that winit PR is making this change to many more crates, slowly turning the scales in favour of `once_cell` in most dependency trees. Furthermore `lazy_static` hasn't published any updates for 3 years. See also [the `once_cell` F.A.Q.]. In addition "constant" `Vec`tor allocations don't need to be wrapped in a `lazy_static!` macro call at all but can be replaced with true `const` slices (or `const` sized arrays, but those are slightly more tedious to construct). [motivation in winit]: rust-windowing/winit#2313 [proposed for inclusion in `std`]: rust-lang/rust#74465 [the `once_cell` F.A.Q.]: https://docs.rs/once_cell/latest/once_cell/#faq

Piggybacking on the [motivation in winit]: `lazy_static!` is a macro whereas `once_cell` achieves the same using generics. Its implementation has also been [proposed for inclusion in `std`], making it easier to switch to a standardized version if/when that happens. The author of that winit PR is making this change to many more crates, slowly turning the scales in favour of `once_cell` in most dependency trees (despite this being a `dev-dependency` used in tests exclusively, hence not relevant for downstream crates). Furthermore `lazy_static` hasn't published any updates for 3 years. See also [the `once_cell` F.A.Q.]. [motivation in winit]: rust-windowing/winit#2313 [proposed for inclusion in `std`]: rust-lang/rust#74465 [the `once_cell` F.A.Q.]: https://docs.rs/once_cell/latest/once_cell/#faq

… slices (#471) Piggybacking on the [motivation in winit]: `lazy_static!` is a macro whereas `once_cell` achieves the same using generics. Its implementation has also been [proposed for inclusion in `std`], making it easier to switch to a standardized version if/when that happens. The author of that winit PR is making this change to many more crates, slowly turning the scales in favour of `once_cell` in most dependency trees. Furthermore `lazy_static` hasn't published any updates for 3 years. See also [the `once_cell` F.A.Q.]. In addition "constant" `Vec`tor allocations don't need to be wrapped in a `lazy_static!` macro call at all but can be replaced with true `const` slices (or `const` sized arrays, but those are slightly more tedious to construct). [motivation in winit]: rust-windowing/winit#2313 [proposed for inclusion in `std`]: rust-lang/rust#74465 [the `once_cell` F.A.Q.]: https://docs.rs/once_cell/latest/once_cell/#faq

Piggybacking on the [motivation in winit]: `lazy_static!` is a macro whereas `once_cell` achieves the same using generics. Its implementation has also been [proposed for inclusion in `std`], making it easier for us to switch to a standardized version if/when that happens. The author of that winit PR is making this change to many more crates, slowly turning the scales in favour of `once_cell` in our dependency tree too. Furthermore `lazy_static` hasn't published any updates for 3 years, and the new syntax is closer for dropping this wrapping completely when the necessary constructors become `const` (i.e. switching to `parking_lot` will give us a [`const fn new()` on `RwLock`]) or this feature lands in stable `std`. [motivation in winit]: rust-windowing/winit#2313 [proposed for inclusion in `std`]: rust-lang/rust#74465 [`const fn new()` on `RwLock`]: https://docs.rs/lock_api/latest/lock_api/struct.RwLock.html#method.new

Migrate from lazy_static to once_cell

7a6d844

filnet reviewed Jun 6, 2022

View reviewed changes

src/platform_impl/linux/x11/monitor.rs Outdated Show resolved Hide resolved

filnet reviewed Jun 6, 2022

View reviewed changes

src/platform_impl/linux/x11/util/wm.rs Outdated Show resolved Hide resolved

madsmtm added the S - maintenance Repaying technical debt label Jun 6, 2022

madsmtm approved these changes Jun 6, 2022

View reviewed changes

maroider reviewed Jun 7, 2022

View reviewed changes

src/platform_impl/macos/app_state.rs Outdated Show resolved Hide resolved

src/platform_impl/macos/window.rs Outdated Show resolved Hide resolved

kchibisov reviewed Jun 7, 2022

View reviewed changes

src/platform_impl/linux/x11/ime/input_method.rs Outdated Show resolved Hide resolved

kchibisov reviewed Jun 7, 2022

View reviewed changes

src/platform_impl/linux/x11/ime/input_method.rs Outdated Show resolved Hide resolved

kchibisov reviewed Jun 7, 2022

View reviewed changes

src/platform_impl/linux/mod.rs Outdated Show resolved Hide resolved

kchibisov requested changes Jun 7, 2022

View reviewed changes

james7132 added 5 commits June 8, 2022 04:08

Revert some of the unintentional cargo fmt --all moves

32cbcfc

Fix formatting errors and fix the import order for other files

7c4a803

Undo more spurious moves

a1cd124

Fix formatting

8e0266b

Fix up more imports

818b727

james7132 requested review from kchibisov and maroider June 8, 2022 11:30

maroider approved these changes Jun 8, 2022

View reviewed changes

kchibisov approved these changes Jun 8, 2022

View reviewed changes

kchibisov merged commit 2c01e9e into rust-windowing:master Jun 8, 2022

MarijnS95 mentioned this pull request Jun 10, 2022

ndk-glue: Replace ancient lazy_static crate with once_cell rust-mobile/ndk#285

Merged

MarijnS95 mentioned this pull request Jun 11, 2022

Replace ancient lazy_static crate with once_cell harryfei/which-rs#58

Merged

MarijnS95 mentioned this pull request Jun 11, 2022

Replace ancient lazy_static crate with once_cell or const slices getsentry/sentry-rust#471

Merged

MarijnS95 mentioned this pull request Jun 12, 2022

Replace ancient lazy_static crate with once_cell app-dirs-rs/app_dirs2#30

Merged

james7132 mentioned this pull request Feb 24, 2024

Remove once_cell dependency #3520

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate from lazy_static to once_cell #2313

Migrate from lazy_static to once_cell #2313

james7132 commented Jun 5, 2022

madsmtm left a comment

maroider left a comment •

edited

Loading

kchibisov left a comment

james7132 commented Jun 8, 2022

madsmtm commented Jun 8, 2022

maroider left a comment

MarijnS95 commented Jun 9, 2022

kchibisov commented Jun 9, 2022

MarijnS95 commented Jun 10, 2022

Migrate from lazy_static to once_cell #2313

Migrate from lazy_static to once_cell #2313

Conversation

james7132 commented Jun 5, 2022

Motivation

Solution

madsmtm left a comment

Choose a reason for hiding this comment

maroider left a comment • edited Loading

Choose a reason for hiding this comment

kchibisov left a comment

Choose a reason for hiding this comment

james7132 commented Jun 8, 2022

madsmtm commented Jun 8, 2022

maroider left a comment

Choose a reason for hiding this comment

MarijnS95 commented Jun 9, 2022

kchibisov commented Jun 9, 2022

MarijnS95 commented Jun 10, 2022

maroider left a comment •

edited

Loading