Skip to content

Time macros: use tryfinally macro approach#39138

Closed
IanButterworth wants to merge 3 commits intoJuliaLang:masterfrom
IanButterworth:ib/time_tryfinally_macro
Closed

Time macros: use tryfinally macro approach#39138
IanButterworth wants to merge 3 commits intoJuliaLang:masterfrom
IanButterworth:ib/time_tryfinally_macro

Conversation

@IanButterworth
Copy link
Copy Markdown
Member

@IanButterworth IanButterworth commented Jan 7, 2021

@KristofferC's suggestion in #39133 of the @tryfinally macro approach is a neat way to maintain scope while ensuring the finalizer is run

I verified that the finalizer is run in each of these:

julia> @time y = 1
  0.000005 seconds
1

julia> y
1

julia> @time error()
ERROR: 
Stacktrace:
 [1] error()
   @ Base ./error.jl:42
 [2] top-level scope
   @ ~/Documents/GitHub/julia/base/timing.jl:213 [inlined]
 [3] top-level scope
   @ ./REPL[31]:0

julia> x = rand(3,3);

julia> Threads.@threads for i in 1:20 @time x * x; end
  0.565114 seconds (2.36 M allocations: 126.880 MiB, 4.93% gc time, 99.95% compilation time)
  0.566743 seconds (2.36 M allocations: 126.893 MiB, 4.92% gc time, 0.00% compilation time)
  0.000002 seconds (1 allocation: 160 bytes)
  0.565122 seconds (2.36 M allocations: 126.882 MiB, 4.93% gc time, 0.00% compilation time)
  0.568685 seconds (2.36 M allocations: 126.907 MiB, 4.90% gc time, 0.00% compilation time)
  0.566732 seconds (2.36 M allocations: 126.890 MiB, 4.92% gc time, 0.00% compilation time)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)
  0.565109 seconds (2.36 M allocations: 126.879 MiB, 4.93% gc time, 0.01% compilation time)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000000 seconds (1 allocation: 160 bytes)
  0.000001 seconds (2 allocations: 192 bytes)
  0.000001 seconds (1 allocation: 160 bytes)

This new @tryfinally macro could be pretty useful! Perhaps in a future PR @trycatch could be added and both exported

julia> Base.@tryfinally(xx = 1, println("finalizer"))
finalizer
1

julia> xx
1

julia> Base.@tryfinally(error(), println("finalizer"))
finalizer
ERROR: 
Stacktrace:
 [1] error()
   @ Base ./error.jl:42
 [2] top-level scope
   @ REPL[37]:1

@IanButterworth
Copy link
Copy Markdown
Member Author

IanButterworth commented Jan 12, 2021

If this is preferable, it should probably go into the next 1.6 beta etc. @JeffBezanson ?

@IanButterworth IanButterworth mentioned this pull request Jan 12, 2021
Closed
@JeffBezanson
Copy link
Copy Markdown
Member

JeffBezanson commented Jan 13, 2021

I don't think it's worth switching to this. tryfinally is an intermediate form that we should not use in source ASTs.

@IanButterworth
Copy link
Copy Markdown
Member Author

IanButterworth commented Jan 15, 2021

Ok!
Just to tie the two together this intended to use the same mechanism #39217

@NHDaly
Copy link
Copy Markdown
Member

NHDaly commented Aug 2, 2021

Interesting.

This looks great. Makes sense that we want to wait for #39217.

In the meantime, just to be sure everyone knows, we realized that the current @time macro implementation turns off compilation measurement whenever anyone throws an exception, even one that's handled:
https://github.com/JuliaLang/julia/pull/41733/files#r681256138

julia> @time begin
           # The following exception disables compilation time measurement
           try throw("ha HAH!") catch end
           @eval module M ; f(x) = 2*x ; end
           @eval M.f(2)
       end
  0.006116 seconds (2.17 k allocations: 156.874 KiB)
4

So this fix would definitely be nice. 😊

Does it still make sense to wait for #39217? How's the progress there?

This was referenced Aug 18, 2021
Merged
Merged
@Sacha0 Sacha0 mentioned this pull request Aug 26, 2021
Closed
@IanButterworth IanButterworth deleted the ib/time_tryfinally_macro branch March 4, 2022 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@IanButterworth @JeffBezanson @NHDaly