Add globally registered timers

[morris25]

Closes #134.

Adds get_timer function to register and retrieve timers. This allows users to access timers for different workflows without having to pass them through intermediate packages.

Example use case:

module ReadOnlyDB

using TimerOutputs

function get_data(...)
     db_timer = get_timer("DBAccess")  # Used in all DB packages
     @timeit db_timer "fetch" ...
end
end

using Simulation  # uses PkgA which uses PkgB which uses several DB packages
using TimerOutputs
simulate(...)

print_timer(get_timer("DBAccess"))  # keep track of DB efficiency

[KristofferC]

I don't think your example will work:

julia> TimerOutputs._timers
Dict{String, TimerOutput} with 1 entry:
  "Default" =>  ──────────────────────────────────────────────────────────────────…

julia> using ReadOnlyDB

julia> TimerOutputs._timers
Dict{String, TimerOutput} with 1 entry:
  "Default" =>  ──────────────────────────────────────────────────────────────────…

You would need to retrieve the timer in __init__ and assign it somewhere, I think.

[morris25]

I don't think your example will work:

julia> TimerOutputs._timers
Dict{String, TimerOutput} with 1 entry:
  "Default" =>  ──────────────────────────────────────────────────────────────────…

julia> using ReadOnlyDB

julia> TimerOutputs._timers
Dict{String, TimerOutput} with 1 entry:
  "Default" =>  ──────────────────────────────────────────────────────────────────…

You would need to retrieve the timer in __init__ and assign it somewhere, I think.

You're right. I've simplified the example to get the timer locally rather than setting a const. That's what I've been doing in my code anyway. We could add something similar to Memento.register to set registered timers but I'm not sure it's necessary.

Edit: I tried adding a register function and realized that we would run into weird situations when the timer gets disabled if there are multiple copies floating around. It's probably best to only use a timer from _timers locally.

[KristofferC]

I think this is ok (still needs some docs in the README). But this shouldn't really be used by packages since it basically puts everything in a shared namespace and if packages use it you might get accidental collisions. This is why I am feeling this is perhaps not the ideal solution to the problem.

[morris25]

I think this is ok (still needs some docs in the README). But this shouldn't really be used by packages since it basically puts everything in a shared namespace and if packages use it you might get accidental collisions. This is why I am feeling this is perhaps not the ideal solution to the problem.

Yeah, this solution is meant for really limited use. I basically want to keep a human-readable trace of the runtimes of a couple components within our system on live runs. As is stands, the current solution good enough for the situation I need it for which is essentially a couple named default timers.

If you have any ideas for alternate solutions to the problem I'd be happy to try them out! I'll add some documentation in the meantime.

[fredrikekre]

Why not just define the named timers as global variables?

[KristofferC]

Presumably because you want to share them between multiple packages so one uses TimerOutputs.jl as a "storage" because all packages have access to it.

[morris25]

We have a system that uses ~300 packages and want to keep track of live runtimes for a few components in around 5 of them for business reasons. Our POC uses the default timer but that is pretty dangerous.

[morris25]

The docs have been updated for a while now if anyone has time for re-review.

[codecov-commenter]

Codecov Report

Merging #135 (3c49843) into master (90a5ee5) will decrease coverage by 4.86%. The diff coverage is 81.08%.

@@            Coverage Diff             @@
##           master     #135      +/-   ##
==========================================
- Coverage   93.10%   88.23%   -4.87%     
==========================================
  Files           4        5       +1     
  Lines         377      408      +31     
==========================================
+ Hits          351      360       +9     
- Misses         26       48      +22     

Impacted Files	Coverage Δ
src/TimerOutputs.jl	100.00% <ø> (ø)
src/precompile.jl	0.00% <0.00%> (ø)
src/TimerOutput.jl	86.69% <95.00%> (-3.86%)	:arrow_down:
src/show.jl	95.49% <100.00%> (+0.16%)	:arrow_up:
src/utilities.jl	95.23% <0.00%> (-1.59%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c3a4dde...3c49843. Read the comment docs.

[fredrikekre]

One could perhaps suggest in the docs or even enforce that those timers are fetched using the package UUID?

[KristofferC]

One could perhaps suggest in the docs or even enforce that those timers are fetched using the package UUID?

I think it is fine to warn.