Block const-prop in CoreLogging #39921
Conversation
It turns out that constant-propagation contributed ~50ms of latency to Requires.jl. Worse, it does it for each time it's used thanks to specialization by constant-propagation. This prevents specialization on the Symbol/Module pair in `env_override_minlevel`.
| @@ -474,7 +474,8 @@ function current_logstate() | |||
| end | |||
|
|
|||
| # helper function to get the current logger, if enabled for the specified message type | |||
| @noinline function current_logger_for_env(std_level::LogLevel, group, _module) | |||
| @noinline function current_logger_for_env(std_level::LogLevel, group::Union{String,Symbol}, _module::Module) | |||
| @nospecialize # block const-prop | |||
There was a problem hiding this comment.
I don't think this actually blocks constant prop in inference (does it?) --- but it probably should!
There was a problem hiding this comment.
Hmm, maybe not reliably. But it it seems to help in some cases. E.g., I've used it several times on keyword arguments to prevent #38983 (comment).
There was definitely a big difference in inference timing here and the const-prop didn't show up any more in @snoopi_deep recordings, but you're right that it doesn't always seem to prevent const-prop.
EDIT: now I'm not so sure it helps in this specific case.
There was a problem hiding this comment.
I don't think these types are correct either. I'm pretty sure ::Any was right
There was a problem hiding this comment.
The test failures definitely say we need at least Nothing.
|
Hmm, I must have conflated this with other changes I was making in JuliaPackaging/Requires.jl#101. Now I really don't see any difference in inference. Unless it's LLVM time? |
|
Happy to close if this is useless. |
It turns out that constant-propagation contributed ~50ms of latency to
Requires.jl. Worse, it does it for each time it's used thanks to
specialization by constant-propagation. This prevents const-specialization
on the Symbol/Module pair in
env_override_minlevel.