Logging: Improve threadsafety #57591
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
IanButterworth
added
multithreading
IanButterworth
changed the title
SimpleLogger and ConsoleLogger threadsafety
IanButterworth
force-pushed
the
ib/logging_lock
branch
from
6462f96 to
a40e867
Compare
IanButterworth
force-pushed
the
ib/logging_lock
branch
from
a40e867 to
8e5a2e1
Compare
IanButterworth
commented
Mar 1, 2025
IanButterworth
force-pushed
the
ib/logging_lock
branch
3 times, most recently
from
110a64c to
bbfaffe
Compare
|
This may need some comments and it needs documentation, since adding a lock can interact poorly with other existing locks, so users need to be aware of the risk, to be able to structure their code safely |
This was referenced Mar 11, 2025
Merged
IanButterworth
force-pushed
the
ib/logging_lock
branch
from
bbfaffe to
a0b7b27
Compare
|
@vtjnash I added some explanation to the docs and a comment, and went as far as stating thread-safety. Do you think the doc additions are sufficient detail to allow users to understand risks or should it be more explicit? |
|
Bump. This seems important. |
|
@vtjnash what do you think? |
|
Bump @vtjnash |
vtjnash
reviewed
Apr 3, 2025
| @@ -171,7 +172,7 @@ Alias for [`LogLevel(1_000_001)`](@ref LogLevel). | |||
| const AboveMaxLevel = LogLevel( 1000001) | |||
|
|
|||
| # Global log limiting mechanism for super fast but inflexible global log limiting. | |||
| const _min_enabled_level = Ref{LogLevel}(Debug) | |||
| const _min_enabled_level = Threads.Atomic{Int32}(Debug) | |||
There was a problem hiding this comment.
Just adding the discussion from the multithreading call that Threads.Atomic can only take certain types.
and use for maxlog and stream writes
(cherry picked from commit 349709a)
IanButterworth
force-pushed
the
ib/logging_lock
branch
from
a0b7b27 to
019258a
Compare
51 tasks
vtjnash
approved these changes
Apr 4, 2025
kpamnany
pushed a commit
to RelationalAI/julia
that referenced
this pull request
Apr 7, 2025
Closes JuliaLang#57376 Closes JuliaLang#34037 - Adds a lock in `SimpleLogger` and `ConsoleLogger` for use on maxlog tracking and stream writes to improve threadsafety. Closely similar to JuliaLang#54497 - Turns the internal `_min_enabled_level` into a `Threads.Atomic`. There are [some direct interactions](https://juliahub.com/ui/Search?type=code&q=_min_enabled_level&w=true) to this internal in the ecosystem, but they should still work ``` julia> Base.CoreLogging._min_enabled_level[] = Logging.Info+1 LogLevel(1) ``` - Brings tests over from JuliaLang#57448 Performance seems highly similar: ``` julia> @time for i in 1:10000 @info "foo" maxlog=10000000 end [ Info: foo ... 0.481446 seconds (1.33 M allocations: 89.226 MiB, 0.49% gc time) ``` ``` 0.477235 seconds (1.31 M allocations: 79.002 MiB, 1.77% gc time) ```
3 tasks
KristofferC
pushed a commit
that referenced
this pull request
Apr 9, 2025
Closes #57376 Closes #34037 - Adds a lock in `SimpleLogger` and `ConsoleLogger` for use on maxlog tracking and stream writes to improve threadsafety. Closely similar to #54497 - Turns the internal `_min_enabled_level` into a `Threads.Atomic`. There are [some direct interactions](https://juliahub.com/ui/Search?type=code&q=_min_enabled_level&w=true) to this internal in the ecosystem, but they should still work ``` julia> Base.CoreLogging._min_enabled_level[] = Logging.Info+1 LogLevel(1) ``` - Brings tests over from #57448 Performance seems highly similar: ### Master ``` julia> @time for i in 1:10000 @info "foo" maxlog=10000000 end [ Info: foo ... 0.481446 seconds (1.33 M allocations: 89.226 MiB, 0.49% gc time) ``` ### This PR ``` 0.477235 seconds (1.31 M allocations: 79.002 MiB, 1.77% gc time) ``` (cherry picked from commit 9af9650)
KristofferC
removed
the
backport 1.12
61 tasks
KristofferC
pushed a commit
that referenced
this pull request
Jun 4, 2025
Closes #57376 Closes #34037 - Adds a lock in `SimpleLogger` and `ConsoleLogger` for use on maxlog tracking and stream writes to improve threadsafety. Closely similar to #54497 - Turns the internal `_min_enabled_level` into a `Threads.Atomic`. There are [some direct interactions](https://juliahub.com/ui/Search?type=code&q=_min_enabled_level&w=true) to this internal in the ecosystem, but they should still work ``` julia> Base.CoreLogging._min_enabled_level[] = Logging.Info+1 LogLevel(1) ``` - Brings tests over from #57448 Performance seems highly similar: ### Master ``` julia> @time for i in 1:10000 @info "foo" maxlog=10000000 end [ Info: foo ... 0.481446 seconds (1.33 M allocations: 89.226 MiB, 0.49% gc time) ``` ### This PR ``` 0.477235 seconds (1.31 M allocations: 79.002 MiB, 1.77% gc time) ``` (cherry picked from commit 9af9650)
KristofferC
pushed a commit
that referenced
this pull request
Jun 5, 2025
Closes #57376 Closes #34037 - Adds a lock in `SimpleLogger` and `ConsoleLogger` for use on maxlog tracking and stream writes to improve threadsafety. Closely similar to #54497 - Turns the internal `_min_enabled_level` into a `Threads.Atomic`. There are [some direct interactions](https://juliahub.com/ui/Search?type=code&q=_min_enabled_level&w=true) to this internal in the ecosystem, but they should still work ``` julia> Base.CoreLogging._min_enabled_level[] = Logging.Info+1 LogLevel(1) ``` - Brings tests over from #57448 Performance seems highly similar: ``` julia> @time for i in 1:10000 @info "foo" maxlog=10000000 end [ Info: foo ... 0.481446 seconds (1.33 M allocations: 89.226 MiB, 0.49% gc time) ``` ``` 0.477235 seconds (1.31 M allocations: 79.002 MiB, 1.77% gc time) ``` (cherry picked from commit 9af9650)
KristofferC
pushed a commit
that referenced
this pull request
Jun 5, 2025
Closes #57376 Closes #34037 - Adds a lock in `SimpleLogger` and `ConsoleLogger` for use on maxlog tracking and stream writes to improve threadsafety. Closely similar to #54497 - Turns the internal `_min_enabled_level` into a `Threads.Atomic`. There are [some direct interactions](https://juliahub.com/ui/Search?type=code&q=_min_enabled_level&w=true) to this internal in the ecosystem, but they should still work ``` julia> Base.CoreLogging._min_enabled_level[] = Logging.Info+1 LogLevel(1) ``` - Brings tests over from #57448 Performance seems highly similar: ``` julia> @time for i in 1:10000 @info "foo" maxlog=10000000 end [ Info: foo ... 0.481446 seconds (1.33 M allocations: 89.226 MiB, 0.49% gc time) ``` ``` 0.477235 seconds (1.31 M allocations: 79.002 MiB, 1.77% gc time) ``` (cherry picked from commit 9af9650)
KristofferC
pushed a commit
that referenced
this pull request
Jun 5, 2025
Closes #57376 Closes #34037 - Adds a lock in `SimpleLogger` and `ConsoleLogger` for use on maxlog tracking and stream writes to improve threadsafety. Closely similar to #54497 - Turns the internal `_min_enabled_level` into a `Threads.Atomic`. There are [some direct interactions](https://juliahub.com/ui/Search?type=code&q=_min_enabled_level&w=true) to this internal in the ecosystem, but they should still work ``` julia> Base.CoreLogging._min_enabled_level[] = Logging.Info+1 LogLevel(1) ``` - Brings tests over from #57448 Performance seems highly similar: ``` julia> @time for i in 1:10000 @info "foo" maxlog=10000000 end [ Info: foo ... 0.481446 seconds (1.33 M allocations: 89.226 MiB, 0.49% gc time) ``` ``` 0.477235 seconds (1.31 M allocations: 79.002 MiB, 1.77% gc time) ``` (cherry picked from commit 9af9650)
KristofferC
pushed a commit
that referenced
this pull request
Jun 5, 2025
Closes #57376 Closes #34037 - Adds a lock in `SimpleLogger` and `ConsoleLogger` for use on maxlog tracking and stream writes to improve threadsafety. Closely similar to #54497 - Turns the internal `_min_enabled_level` into a `Threads.Atomic`. There are [some direct interactions](https://juliahub.com/ui/Search?type=code&q=_min_enabled_level&w=true) to this internal in the ecosystem, but they should still work ``` julia> Base.CoreLogging._min_enabled_level[] = Logging.Info+1 LogLevel(1) ``` - Brings tests over from #57448 Performance seems highly similar: ``` julia> @time for i in 1:10000 @info "foo" maxlog=10000000 end [ Info: foo ... 0.481446 seconds (1.33 M allocations: 89.226 MiB, 0.49% gc time) ``` ``` 0.477235 seconds (1.31 M allocations: 79.002 MiB, 1.77% gc time) ``` (cherry picked from commit 9af9650)
KristofferC
pushed a commit
that referenced
this pull request
Jun 5, 2025
Closes #57376 Closes #34037 - Adds a lock in `SimpleLogger` and `ConsoleLogger` for use on maxlog tracking and stream writes to improve threadsafety. Closely similar to #54497 - Turns the internal `_min_enabled_level` into a `Threads.Atomic`. There are [some direct interactions](https://juliahub.com/ui/Search?type=code&q=_min_enabled_level&w=true) to this internal in the ecosystem, but they should still work ``` julia> Base.CoreLogging._min_enabled_level[] = Logging.Info+1 LogLevel(1) ``` - Brings tests over from #57448 Performance seems highly similar: ``` julia> @time for i in 1:10000 @info "foo" maxlog=10000000 end [ Info: foo ... 0.481446 seconds (1.33 M allocations: 89.226 MiB, 0.49% gc time) ``` ``` 0.477235 seconds (1.31 M allocations: 79.002 MiB, 1.77% gc time) ``` (cherry picked from commit 9af9650)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #57376
Closes #34037
Adds a lock in
SimpleLoggerandConsoleLoggerfor use on maxlog tracking and stream writes to improve threadsafety.Closely similar to Make TestLogger thread-safe (introduce a lock) #54497 by @NHDaly
Turns the internal
_min_enabled_levelinto aThreads.Atomic. There are some direct interactions to this internal in the ecosystem, but they should still workPerformance seems highly similar:
Master
This PR