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

fix invalidations from Static.jl #140

Merged
merged 1 commit into from
Sep 6, 2022
Merged

Conversation

ranocha
Copy link
Contributor

@ranocha ranocha commented Aug 26, 2022

This should hopefully fix quite some invalidations coming from Static.jl.

Here is the code:
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.8.0 (2022-08-17)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

(@v1.8) pkg> activate --temp

(jl_PDrBpd) pkg> add Static
    Updating registry at `~/.julia/registries/General.toml`
   Resolving package versions...
    Updating `/tmp/jl_PDrBpd/Project.toml`
  [aedffcd0] + Static v0.7.6
    Updating `/tmp/jl_PDrBpd/Manifest.toml`
  [615f187c] + IfElse v0.1.1
  [aedffcd0] + Static v0.7.6

julia> using SnoopCompileCore

julia> invalidations = @snoopr using Static

julia> using SnoopCompile

julia> trees = invalidation_trees(invalidations)
7-element Vector{SnoopCompile.MethodInvalidations}:
...
 inserting !(::False) in Static at ~/.julia/packages/Static/sVI3g/src/Static.jl:427 invalidated:
...
                 158: signature Tuple{typeof(!), Any} triggered MethodInstance for Tar.var"#read_tarball#47"(::Vector{UInt8}, ::Base.DevNull, ::typeof(Tar.read_tarball), ::Function, ::Function, ::Base.Process) (48 children)

Since

  • !predicate is used here in an if stament - and if statements only accept Bools
  • and it is documented that predicate has to return a Bool

it appears to be sane to assert that predicate returns a Bool here.

@codecov
Copy link

codecov bot commented Aug 26, 2022

Codecov Report

Merging #140 (f95e811) into master (5606269) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #140   +/-   ##
=======================================
  Coverage   97.29%   97.29%           
=======================================
  Files           4        4           
  Lines         775      775           
=======================================
  Hits          754      754           
  Misses         21       21           
Impacted Files Coverage Δ
src/extract.jl 98.11% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ranocha
Copy link
Contributor Author

ranocha commented Aug 30, 2022

I suggest the labels latency and backport-1.8 for this PR.

@ranocha
Copy link
Contributor Author

ranocha commented Sep 1, 2022

Bump. Can we get this into Julia v1.8.1?

1 similar comment
@ranocha
Copy link
Contributor Author

ranocha commented Sep 5, 2022

Bump. Can we get this into Julia v1.8.1?

@staticfloat staticfloat merged commit d5fd14a into JuliaIO:master Sep 6, 2022
@ranocha ranocha deleted the patch-3 branch September 6, 2022 15:42
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.

2 participants