Skip to content
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

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

Merged
merged 3 commits into from
Jan 7, 2021

Conversation

IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Jan 6, 2021

#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

@JeffBezanson JeffBezanson added the backport 1.6 Change should be backported to release-1.6 label Jan 7, 2021
src/builtins.c Outdated Show resolved Hide resolved
@IanButterworth
Copy link
Member Author

tester_linux64 is now passing again

@KristofferC
Copy link
Member

KristofferC commented Jan 7, 2021

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
Member Author

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

@KristofferC
Copy link
Member

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
Member Author

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
* 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 pushed a commit that referenced this pull request Jan 8, 2021
* 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
* 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
…ng#39133)

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

* add scope tests for at-time and aat-timev
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
* 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants