Precompilation broken on Julia 1.9-rc1

palday commented 1 year ago

The lines impacted are

https://github.com/apache/arrow-julia/blob/63d2c9d3ca4539a0ea831ae8ecafa71b051d475d/src/ArrowTypes/src/ArrowTypes.jl#L337-L339

(@v1.9) pkg> activate --temp
  Activating new project at `/var/folders/yy/nyj87tsn7093bb7d84rl64rh0000gp/T/jl_rxXyRC`

(jl_rxXyRC) pkg> add ArrowTypes#main
     Cloning git-repo `https://github.com/apache/arrow-julia.git`
    Updating git-repo `https://github.com/apache/arrow-julia.git`
    Updating registry at `~/.julia/registries/Beacon`
    Updating git-repo `https://github.com/beacon-biosignals/BeaconRegistry.git`
    Updating registry at `~/.julia/registries/General.toml`
   Resolving package versions...
    Updating `/private/var/folders/yy/nyj87tsn7093bb7d84rl64rh0000gp/T/jl_rxXyRC/Project.toml`
  [31f734f8] + ArrowTypes v2.0.2 `https://github.com/apache/arrow-julia.git:src/ArrowTypes#main`
    Updating `/private/var/folders/yy/nyj87tsn7093bb7d84rl64rh0000gp/T/jl_rxXyRC/Manifest.toml`
  [31f734f8] + ArrowTypes v2.0.2 `https://github.com/apache/arrow-julia.git:src/ArrowTypes#main`
  [9a3f8284] + Random
  [ea8e919c] + SHA v0.7.0
  [9e88b42a] + Serialization
  [6462fe0b] + Sockets
  [cf7118a7] + UUIDs
Precompiling environment...
  1 dependency successfully precompiled in 1 seconds
  1 dependency had warnings during precompilation:
┌ ArrowTypes [31f734f8-188a-4ce0-8406-c8a06bd891cd]
│  WARNING: Method definition default(Type{Union{Base.Missing, Nothing, T}}) where {T} in module ArrowTypes at /Users/palday/.julia/packages/ArrowTypes/bf5DB/src/ArrowTypes.jl:337 overwritten at /Users/palday/.julia/packages/ArrowTypes/bf5DB/src/ArrowTypes.jl:339.
│    ** incremental compilation may be fatally broken for this module **

This may be a Julia issue -- if default(::Type{Union{T, Missing, Nothing}}) overwrites default(::Type{Union{T, Missing}}), it feels like it should also overwrite default(::Type{Union{T, Nothing}}) and besides there's no <: in there so this shouldn't be handled as covariant anyway.

quinnj commented 1 year ago

Hmmmm.....I do see the issue on 1.9-rc1, but not on a recent julia#master (as of 4 days ago).

palday commented 1 year ago

bisected it to

25bad181eb184bce7d3a32a18699fe9f9f9a9325 is the first bad commit
commit 25bad181eb184bce7d3a32a18699fe9f9f9a9325
Author: Jeff Bezanson <jeff.bezanson@gmail.com>
Date:   Tue Jan 24 09:05:18 2023 -0500

    fix #47658, state stack overflow on unions containing typevars (#48221)

    (cherry picked from commit 596ce6542624e9b8c3782b19936e2226f307e118)

 src/subtype.c   | 34 ++++++++++++++++++++++++++++++++++
 test/subtype.jl | 24 +++++++++++++++++++++++-
 2 files changed, 57 insertions(+), 1 deletion(-)

palday commented 1 year ago

(bisecting between 1.9-alpha1 and current head of release-1.9). I'll also take a look at bisecting on master to see what fixed it again there

palday commented 1 year ago

On Julia master, this seems to be fixed by:

commit 96acd4d4e90647caab17af0f689723f5a2594cfd
Author: N5N3 <2642243996@qq.com>
Date:   Thu Feb 2 11:41:26 2023 +0800

    Subtype: avoid false alarm caused by eager `forall_exists_subtype`. (#48441)

    * Avoid earsing `Runion` within nested `forall_exists_subtype`

    If `Runion.more != 0` we‘d better not erase the local `Runion` as we need it if the subtyping fails after.

    This commit replaces `forall_exists_subtype` with a local version.
    It first tries `forall_exists_subtype` and estimates the "problem scale".
    If subtyping fails and the scale looks small then it switches to the slow path.

    TODO: At present, the "problem scale" only counts the number of checked `Lunion`s.
    But perhaps we need a more accurate result (e.g. sum of `Runion.depth`)

    * Change the reversed subtyping into a local check.

    Make sure we don't forget the bound in `env`.
    (And we can fuse `local_forall_exists_subtype`)

    * Optimization for non-union invariant parameter.

 src/subtype.c   | 119 ++++++++++++++++++++++++++++++++++++++------------------
 test/subtype.jl |  13 +++++--
 2 files changed, 91 insertions(+), 41 deletions(-)

palday commented 1 year ago

The upstream fix has been cherry-picked onto the backports-release-1.9 branch. Building on that fixes the issue.

apache / arrow-julia

Precompilation broken on Julia 1.9-rc1 #391