-
Notifications
You must be signed in to change notification settings - Fork 14
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
First pass at improving load times #164
Conversation
Due to calling `config!` and `register` in the `__init__` function our load times were on the order of a few seconds. To address this issue we've cached some default values (e.g., formatter, localzone) and added some precompile logic. Before: ``` julia> @time using Memento 1.289233 seconds (3.60 M allocations: 186.269 MiB, 3.61% gc time) ``` After: ``` julia> @time using Memento 0.672002 seconds (1.13 M allocations: 64.016 MiB, 2.14% gc time) ``` NOTE: While `@time` isn't very reliable for times, you can see that we've reduced the number of allocations and amount of memory being allocated by about 1/2 and 1/3 respectively.
@@ -25,11 +25,11 @@ Ex) "[{level} | {name}]: {msg}" will print message of the form | |||
struct DefaultFormatter <: Formatter | |||
fmt_str::AbstractString | |||
tokens::Vector{Pair{Symbol, Bool}} | |||
output_tz::Dates.TimeZone | |||
output_tz::Union{Dates.TimeZone, Nothing} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beyond the scope of this PR maybe:
I wonder if we should have a timezone in TimeZones.jl called User
or UserLocal
,
which has the behavour of that having this as nothing
does; where it is a caching version of localzone
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I was talking to Curtis a bit about it, but I'd have to think more about how it should work and if that has some potential downfalls. Looks like this caching behaviour might be causing issues on Linux.
Codecov Report
@@ Coverage Diff @@
## master #164 +/- ##
==========================================
- Coverage 96.99% 95.19% -1.81%
==========================================
Files 12 13 +1
Lines 333 333
==========================================
- Hits 323 317 -6
- Misses 10 16 +6
Continue to review full report at Codecov.
|
…ements (single precompile and lazy localzone call.
FWIW, minimizing the changes is a little worse, but not too bad:
|
Due to calling
config!
andregister
in the__init__
function our load times were on the order of a few seconds.To address this issue we've cached some default values (e.g., formatter, localzone) and added some precompile logic.
Before:
After:
NOTE: While
@time
isn't very reliable for times, you can see that we've reduced the number of allocations and amount of memory being allocated by about 1/2 and 1/3 respectively.Similar to #163, but doesn't require lowering the optimization level.