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

release-1.7: Backports for 1.7.0/1.7.0-rc3 #42765

Merged
merged 72 commits into from
Nov 13, 2021
Merged

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Oct 22, 2021

Backported PRs:

Backported CI PRs:

Need manual backport:

Contains multiple commits, manual intervention needed:

Non-merged PRs with backport label:

vtjnash and others added 7 commits October 19, 2021 15:21
It was observed in TSAN that we might get incorrectly locked mutexes
when this code is inlined, due to pthread_self being normally marked
const.

(cherry-picked from 95fac8f92dZ)
The `_wait2` function is similar to calling `schedule + wait` from
another `@async` task, but optimized, so we want to observe the same
side-effects of making the task sticky to the correct thread (and not
accidentally making it sticky to the later task that handles the event).

Refs #41334 (75858d7)

Co-authored-by: Takafumi Arakaki <aka.tkf@gmail.com>
(cherry picked from commit 6420885)
…tion from 60 seconds to 120 seconds (#42753)

(cherry picked from commit 991d6e6)
Co-authored-by: oscarddssmith <oscar.smith@juliacomputing.com>
(cherry picked from commit 609a4a0)
@KristofferC KristofferC added the release Release management and versioning. label Oct 22, 2021
@KristofferC KristofferC requested a review from a team as a code owner October 22, 2021 18:43
@DilumAluthge DilumAluthge removed the request for review from a team October 22, 2021 18:43
staticfloat and others added 11 commits October 22, 2021 20:46
* Disable `DL_LOAD_PATH` prepending for `@`-paths on Darwin

Many thanks to Randy Rucker from Apple for pointing out that we were
technically relying on undefined behavior in `dyld` for loading things
via `jl_dlopen("@loader_path/@rpath/libfoo.dylib")`, due to the
composition of `dlopen("@rpath/libfoo.dylib")` in our Julia code, and
the `DL_LOAD_PATH` prepending we do in `jl_load_dynamic_library()`.

This PR uses a slightly modified version of a patch emailed to me by
Randy, and should solve
JuliaLang/Downloads.jl#149 on macOS Monterey
where the undefined behavior in `dyld` has changed.

* Apply suggestions from code review

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

* Use `[]` instead of `*`

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
(cherry picked from commit 76c2431)
… in inlining code

This commit includes several code quality improvements in inlining code:
- eliminate excessive specializations around:
  * `item::Pair{Any, Any}` constructions
  * iterations on `Vector{Pair{Any, Any}}`
- replace `Pair{Any, Any}` with new, more explicit data type `InliningCase`
- remove dead code
…st-prop'ed sources

This commit complements #39754 and #39305: implements a logic to use
constant-prop'ed results for inlining at union-split callsite.
Currently it works only for cases when constant-prop' succeeded for all
(union-split) signatures.

> example
```julia
julia> mutable struct X
           # NOTE in order to confuse `fieldtype_tfunc`, we need to have at least two fields with different types
           a::Union{Nothing, Int}
           b::Symbol
       end;

julia> code_typed((X, Union{Nothing,Int})) do x, a
           # this `setproperty` call would be union-split and constant-prop will happen for
           # each signature: inlining would fail if we don't use constant-prop'ed source
           # since the approximated inlining cost of `convert(fieldtype(X, sym), a)` would
           # end up very high if we don't propagate `sym::Const(:a)`
           x.a = a
           x
       end |> only |> first
```

> before this commit
```julia
CodeInfo(
1 ─ %1 = Base.setproperty!::typeof(setproperty!)
│   %2 = (isa)(a, Nothing)::Bool
└──      goto #3 if not %2
2 ─ %4 = π (a, Nothing)
│        invoke %1(_2::X, 🅰️:Symbol, %4::Nothing)::Any
└──      goto #6
3 ─ %7 = (isa)(a, Int64)::Bool
└──      goto #5 if not %7
4 ─ %9 = π (a, Int64)
│        invoke %1(_2::X, 🅰️:Symbol, %9::Int64)::Any
└──      goto #6
5 ─      Core.throw(ErrorException("fatal error in type inference (type bound)"))::Union{}
└──      unreachable
6 ┄      return x
)
```

> after this commit
```julia
CodeInfo(
1 ─ %1 = (isa)(a, Nothing)::Bool
└──      goto #3 if not %1
2 ─      Base.setfield!(x, :a, nothing)::Nothing
└──      goto #6
3 ─ %5 = (isa)(a, Int64)::Bool
└──      goto #5 if not %5
4 ─ %7 = π (a, Int64)
│        Base.setfield!(x, :a, %7)::Int64
└──      goto #6
5 ─      Core.throw(ErrorException("fatal error in type inference (type bound)"))::Union{}
└──      unreachable
6 ┄      return x
)
```
…ver `const_prop_entry_heuristic` (#41882)

Currently our constant-prop' heuristics work in the following way:
1. `const_prop_entry_heuristic`
2. `const_prop_argument_heuristic` & `const_prop_rettype_heuristic`
3. `force_const_prop` custom heuristic & `!const_prop_function_heuristic`
4. `MethodInstance` specialization and `const_prop_methodinstance_heuristic`

This PR changes it so that the step 1. now works like:

1. `force_const_prop` custom heuristic & `const_prop_entry_heuristic`

and the steps 2., 3. and 4. don't change

This change particularly allows us to more forcibly constant-propagate
for `getproperty` and `setproperty!`, and inline them more, e.g.:
```julia
mutable struct Foo
    val
    _::Int
end

function setter(xs)
    for x in xs
        x.val = nothing # `setproperty!` can be inlined with this PR
    end
end
```

It might be useful because now we can intervene into the constant-prop'
heuristic in a more reliable way with the `aggressive_constprop` interface.

I did the simple benchmark below, and it looks like this change doesn't
cause the latency problem for this particular example:
```zsh
~/julia master aviatesk@amdci2 6s
❯ ./usr/bin/julia -e '@time using Plots; @time plot(rand(10,3))'
  3.708500 seconds (7.28 M allocations: 506.128 MiB, 3.45% gc time, 1.13% compilation time)
  2.817794 seconds (3.45 M allocations: 195.127 MiB, 7.84% gc time, 53.76% compilation time)

~/julia avi/forceconstantprop aviatesk@amdci2 6s
❯ ./usr/bin/julia -e '@time using Plots; @time plot(rand(10,3))'
  3.622109 seconds (7.02 M allocations: 481.710 MiB, 4.19% gc time, 1.17% compilation time)
  2.863419 seconds (3.44 M allocations: 194.210 MiB, 8.02% gc time, 53.53% compilation time)
```
use more precision when handling loading lock, merge with TOML lock
(since we typically are needing both, sometimes in unpredictable
orders), and unlock before call most user code

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
(cherry picked from commit 3d4b213)
…gardless of where they are located in the repository (#42803)

(cherry picked from commit 6a386de)
This prevents us from seeing an invalid `IOStream` object from a saved
system image, and also ensures the files are opened once for all
threads.

(cherry picked from commit c74814e)
Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
(cherry picked from commit 404e584)
@JeffBezanson
Copy link
Member

We should remove #41602; getting reports that it still doesn't work. We'll probably need to release 1.7 without it, and try to fix it on master.

@KristofferC
Copy link
Member Author

Ok.

barche and others added 3 commits October 31, 2021 18:30

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
* CI (Buildkite): Update all rootfs images to the latest versions

* Re-sign all of the signed pipelines

(cherry picked from commit 9f52ec0)
vtjnash and others added 5 commits November 9, 2021 09:44
…r if networking is unavailable (#42889)

(cherry picked from commit 31b9fd2)
to make it easier to add more information in the future

(cherry picked from commit 9e3cfc0)
This allows scripts loading this file to know the list of tests.

(cherry picked from commit 82784fe)
@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@KristofferC
Copy link
Member Author

The BasicInterpolations error looks consistent but I can't repro it locally (nor on a similar machine as which PkgEval is run).

@KristofferC KristofferC closed this Nov 9, 2021
@KristofferC KristofferC reopened this Nov 9, 2021
KristofferC and others added 9 commits November 10, 2021 10:54
I am not sure why we ever used round+1 instead of ceil+1, as this is
simply strictly more correct.

(cherry picked from commit d6f59fa)
Without this `TakeWhile` has `eltype` of `Any`

(cherry picked from commit ab0c6dd)
… the `TMPDIR` environment variable while running the test (#43012)

(cherry picked from commit ac2ee4d)
JeffBezanson and others added 2 commits November 13, 2021 14:00
Use `show` instead of `print` or `join`.

(cherry picked from commit 6fbfc4f)
@KristofferC
Copy link
Member Author

RR hangs on this branch for some reason but linux has passed on the linux buildkite without RR so that should be good enough.

@KristofferC KristofferC merged commit dc2f781 into release-1.7 Nov 13, 2021
@KristofferC KristofferC deleted the backports-release-1.7 branch November 13, 2021 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Release management and versioning.
Projects
None yet
Development

Successfully merging this pull request may close these issues.