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

const std::lazy::Lazy is a performance footgun when "shared" between threads #82842

Closed
mejrs opened this issue Mar 6, 2021 · 4 comments
Closed

Comments

@mejrs
Copy link
Contributor

mejrs commented Mar 6, 2021

I have a regex that I only want to initialize once, which is why I wrapped it in a lazy_static. However when using the unstable lazy api (see #74465) instead, sharing it between threads causes the initialization function to run every time.

Playground

Note that if static is used rather than const, it (correctly) fails to compile as it's not Sync.

const SyncLazy has the same issue, but not static SyncLazy, which is the correct thing to use here.

@robojumper
Copy link
Contributor

robojumper commented Mar 6, 2021

Clippy complains:

warning: a `const` item should never be interior mutable
  --> src/main.rs:18:1
   |
18 |   const RE: Lazy<Regex> = Lazy::new(|| {
   |   ^----
   |   |
   |  _make this a static item (maybe with lazy_static)
   | |
19 | |     println!("initializing regex");
20 | |     Regex::new(r"(?P<p>\d+)(?:_)(?P<i>\d+)(?:_)(?P<j>\d+)(?:\.png)").unwrap()
21 | | });
   | |___^
   |
   = note: `#[warn(clippy::declare_interior_mutable_const)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const

warning: a `const` item with interior mutability should not be borrowed
  --> src/main.rs:46:16
   |
46 |     let caps = RE.captures(&text).unwrap();
   |                ^^
   |
   = note: `#[warn(clippy::borrow_interior_mutable_const)]` on by default
   = help: assign this const to a local or static variable, and use the variable here
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const

There are false positives to these lints (https://rust-lang.github.io/rust-clippy/master/#interior_mutable_const), which is why these aren't deny-by-default and not in rustc.
"Make this a static item (maybe with lazy_static)" is arguably misleading here, but this could be fixed in clippy.

@sfackler
Copy link
Member

sfackler commented Mar 6, 2021

There is no sharing of anything with a const - they act sort of like #defines, where their definition is pasted in wherever they are used.

@mejrs mejrs changed the title const std::lazy::Lazy is a performance footgun when shared between threads const std::lazy::Lazy is a performance footgun when "shared" between threads Mar 6, 2021
@mejrs
Copy link
Contributor Author

mejrs commented Mar 6, 2021

There is no sharing of anything with a const - they act sort of like #defines, where their definition is pasted in wherever they are used.

That difference wasn't clear to me - thanks for clearing that up. I was somewhat puzzled by it since (as a newbie) it seems const and static can be used interchangably with most primitives.

There are false positives to these lints (https://rust-lang.github.io/rust-clippy/master/#interior_mutable_const), which is why these aren't deny-by-default and not in rustc.
"Make this a static item (maybe with lazy_static)" is arguably misleading here, but this could be fixed in clippy.

That would help a lot.

Although, I can't think of an actual use case for const Lazy - is there any chance this could be a rustc lint? Unusing iterators and ignoring Results are rustc lints; similarly I believe anyone trying to use a const Lazy is making a mistake.

@matklad
Copy link
Member

matklad commented Mar 8, 2021

I think this is a duplicate of #40543, so closing.

I hate how I had opened that issue a while back, and then went ahead with creating a library which now falls victim of it :)

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

4 participants