Skip to content

Remove try from at-time and close compile timer during throw#39133

Merged
KristofferC merged 3 commits intoJuliaLang:masterfrom
IanButterworth:ib/time_remove_try
Jan 7, 2021
Merged

Remove try from at-time and close compile timer during throw#39133
KristofferC merged 3 commits intoJuliaLang:masterfrom
IanButterworth:ib/time_remove_try

Conversation

@IanButterworth
Copy link
Copy Markdown
Member

@IanButterworth IanButterworth commented Jan 6, 2021 •
edited
Loading

#38915 accidentally swallowed assignments due to the try-finally scope

julia> @time x = 1
  0.000001 seconds
1
julia> x
ERROR: UndefVarError: x not defined

this removes the try and disables the timer during every throw

julia> @time x = 1
  0.000000 seconds
1

julia> x
1

And maintains the thread-locality introduced in #38915

julia> x = rand(3,3);

julia> Threads.@threads for i in 1:20 @time x * x; end
  0.652161 seconds (2.36 M allocations: 126.891 MiB, 16.46% gc time, 99.95% compilation time)
  0.655488 seconds (2.36 M allocations: 126.904 MiB, 16.38% gc time, 0.00% compilation time)
  0.652135 seconds (2.36 M allocations: 126.888 MiB, 16.46% gc time, 0.00% compilation time)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)
  0.655499 seconds (2.36 M allocations: 126.912 MiB, 16.38% gc time, 0.00% compilation time)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)
  0.655506 seconds (2.36 M allocations: 126.919 MiB, 16.37% gc time, 0.00% compilation time)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)
  0.653652 seconds (2.36 M allocations: 126.896 MiB, 16.42% 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.000002 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)
  0.000001 seconds (1 allocation: 160 bytes)


julia> @time error(ErrorException("hello"))
ERROR: ErrorException("hello")
Stacktrace:
 [1] error(s::ErrorException)
   @ Base ./error.jl:42
 [2] top-level scope
   @ ./timing.jl:206 [inlined]
 [3] top-level scope
   @ ./REPL[7]:0

cc. @vtjnash @ericphanson

@IanButterworth IanButterworth mentioned this pull request Jan 6, 2021
Merged
@JeffBezanson JeffBezanson added the backport 1.6 Change should be backported to release-1.6 label Jan 7, 2021
JeffBezanson
JeffBezanson reviewed Jan 7, 2021
View reviewed changes
@IanButterworth
Copy link
Copy Markdown
Member Author

IanButterworth commented Jan 7, 2021

tester_linux64 is now passing again

@KristofferC
Copy link
Copy Markdown
Member

KristofferC commented Jan 7, 2021 •
edited
Loading

I think you could use something like:

 macro try_finally(ex, fin)
    Expr(:tryfinally,
       :(local val = $(esc(ex))),
       :($(esc(fin)))
       )
end
julia> @try_finally x = 3 println("finalizing")
finalizing
3

julia> x
3

julia> @try_finally error() println("finalizing")
finalizing
ERROR: 
Stacktrace:
 [1] error() at ./error.jl:42
 [2] top-level scope at REPL[4]:1

but this probably works as well.

@IanButterworth
Copy link
Copy Markdown
Member Author

IanButterworth commented Jan 7, 2021

That's neat.. @JeffBezanson what would be better?

@KristofferC
Copy link
Copy Markdown
Member

KristofferC commented Jan 7, 2021

I'll merge this to get CI working properly again, and then we can discuss the exact implementation (using Expr(:tryfinally, ...) vs this).

@KristofferC KristofferC merged commit 03957db into JuliaLang:master Jan 7, 2021
@IanButterworth
Copy link
Copy Markdown
Member Author

IanButterworth commented Jan 7, 2021

Great, thanks. I'll prepare a PR for the alternative idea

@IanButterworth IanButterworth deleted the ib/time_remove_try branch January 7, 2021 15:37
KristofferC pushed a commit that referenced this pull request Jan 7, 2021
@IanButterworth @KristofferC
Remove try from at-time and close compile timer during throw (#39133)
86eb245 
* remove try from at-time and close compile timer during throw

* add scope tests for at-time and aat-timev

(cherry picked from commit 03957db)
@IanButterworth IanButterworth mentioned this pull request Jan 7, 2021
Closed
vtjnash referenced this pull request Jan 7, 2021
@aviatesk
inference: minor typing improvements for InferenceState.stmt_types (#…
5c6e21e 
…39096)
KristofferC pushed a commit that referenced this pull request Jan 8, 2021
@IanButterworth @KristofferC
Remove try from at-time and close compile timer during throw (#39133)
336c71b 
* remove try from at-time and close compile timer during throw

* add scope tests for at-time and aat-timev

(cherry picked from commit 03957db)
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Jan 8, 2021
staticfloat pushed a commit that referenced this pull request Jan 15, 2021
@IanButterworth @staticfloat
Remove try from at-time and close compile timer during throw (#39133)
ef0fbb5 
* remove try from at-time and close compile timer during throw

* add scope tests for at-time and aat-timev

(cherry picked from commit 03957db)
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
@IanButterworth @ElOceanografo
Remove try from at-time and close compile timer during throw (JuliaLa…
5566710 
…ng#39133)

* remove try from at-time and close compile timer during throw

* add scope tests for at-time and aat-timev
This was referenced Aug 18, 2021
Merged
Merged
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
@IanButterworth @staticfloat
Remove try from at-time and close compile timer during throw (#39133)
d2f74e1 
* remove try from at-time and close compile timer during throw

* add scope tests for at-time and aat-timev

(cherry picked from commit 03957db)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vtjnash vtjnash Awaiting requested review from vtjnash

@JeffBezanson JeffBezanson Awaiting requested review from JeffBezanson

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 @KristofferC @JeffBezanson