Skip to content

Fix 1.9 - #757

Merged
IanButterworth merged 11 commits into
JuliaLang:masterfrom
IanButterworth:ib/outputo_fix
Jan 13, 2023
Merged

Fix 1.9#757
IanButterworth merged 11 commits into
JuliaLang:masterfrom
IanButterworth:ib/outputo_fix

Conversation

@IanButterworth

@IanButterworth IanButterworth commented Jan 6, 2023

Copy link
Copy Markdown
Member

Closes #755

#755 was due to the sysimage output process being launched with O2 vs the precompiled O3, so the caches were rejected, entering a state where __init__ are not run and the RNG wasn't seeded. i.e. the test correctly caught the issue

@codecov

codecov Bot commented Jan 6, 2023

Copy link
Copy Markdown

Codecov Report

Merging #757 (2eb0b72) into master (28cb6cb) will decrease coverage by 2.27%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #757      +/-   ##
==========================================
- Coverage   83.33%   81.06%   -2.28%     
==========================================
  Files           3        3              
  Lines         600      660      +60     
==========================================
+ Hits          500      535      +35     
- Misses        100      125      +25     
Impacted Files Coverage Δ
src/PackageCompiler.jl 91.13% <100.00%> (-0.97%) ⬇️
ext/TerminalSpinners.jl 24.46% <0.00%> (-5.80%) ⬇️
src/juliaconfig.jl 82.97% <0.00%> (+3.90%) ⬆️

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

@IanButterworth

Copy link
Copy Markdown
Member Author

The remaining 1.9+ failure is #755

Comment thread src/PackageCompiler.jl Outdated
@IanButterworth IanButterworth changed the title Fix outputo test if pkgimages were used during precomp Fix 1.9+ Jan 7, 2023
@IanButterworth

Copy link
Copy Markdown
Member Author

Locally on julia master I get ERROR: Unable to find compatible target in system image.

┌ Debug: running `/home/ian/Documents/GitHub/julia/usr/bin/julia --color=yes --startup-file=no '--cpu-target=generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)' -O3 --sysimage=/home/ian/Documents/GitHub/julia/usr/lib/julia/sys.so --project=/tmp/jl_0fuMB7/MyApp --output-o=/tmp/jl_WTIp8zq32r.o /tmp/jl_uzSJWPaVwR`
└ @ PackageCompiler ~/Documents/GitHub/PackageCompiler.jl/src/PackageCompiler.jl:389
⠋ [00m:00s] PackageCompiler: compiling incremental system imageERROR: Unable to find compatible target in system image.
✖ [00m:00s] PackageCompiler: compiling incremental system image
filter = false: Error During Test at /home/ian/Documents/GitHub/PackageCompiler.jl/test/runtests.jl:64
  Got exception outside of a @test
  failed process: Process(`/home/ian/Documents/GitHub/julia/usr/bin/julia --color=yes --startup-file=no '--cpu-target=generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)' -O3 --sysimage=/home/ian/Documents/GitHub/julia/usr/lib/julia/sys.so --project=/tmp/jl_0fuMB7/MyApp --output-o=/tmp/jl_WTIp8zq32r.o /tmp/jl_uzSJWPaVwR`, ProcessExited(1)) [1]

@DilumAluthge

Copy link
Copy Markdown
Member

Locally on julia master I get ERROR: Unable to find compatible target in system image.

Does your local Julia master include JuliaLang/julia#48131?

@IanButterworth

Copy link
Copy Markdown
Member Author

Does your local Julia master include JuliaLang/julia#48131?

Yeah it does. I'm on JuliaLang/julia#48168

julia> versioninfo()
Julia Version 1.10.0-DEV.295
Commit bdf976b260* (2023-01-07 17:36 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 32 × AMD Ryzen 9 5950X 16-Core Processor
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-14.0.6 (ORCJIT, znver3)
  Threads: 16 on 32 virtual cores

@DilumAluthge

Copy link
Copy Markdown
Member

Does this mean that JuliaLang/julia#48131 didn't completely fix JuliaLang/julia#48111?

@IanButterworth

IanButterworth commented Jan 8, 2023

Copy link
Copy Markdown
Member Author

I don't know. It didn't happen all the time.

Tests locally, where:

Test Summary:           | Pass  Fail  Error  Total      Time
PackageCompiler.jl      |   56     2      1     59  32m12.9s
  create_sysimage       |    3                   3   1m30.6s
  create_app            |   52     2      1     55  20m42.3s
    incremental = true  |                 1      1   1m10.6s
      filter = false    |                 1      1   1m10.6s
    incremental = false |   52     2            54  19m31.6s
      filter = true     |   26     1            27   9m49.7s
      filter = false    |   26     1            27   9m41.9s

Although I never saw this error

┌ Debug: Rejecting cache file /tmp/jl_aCrjdm/compiled/v1.9/LibSSH2_jll/K6mup_TmZuG.ji for LibSSH2_jll [29816b5a-b9ab-546f-933c-edad1886dfa8] since the flags are mismatched
│   cachefile_flags = 35
│   current_flags = 51
└ @ Base loading.jl:2526

but I did see a lot of

┌ Debug: Rejecting cache file /tmp/jl_LzuULE/compiled/v1.10/MKL_jll/DHEix_r71fR.ji for MKL_jll [856f044c-d86e-5d09-b602-aeab76dc8ba7] since pkgimage can't be loaded on this target
└ @ Base loading.jl:2565

which is checked after the flags check, so flag check appears to be passing locally.

I'm quite confused here..

@IanButterworth

IanButterworth commented Jan 8, 2023

Copy link
Copy Markdown
Member Author

If it's informative I dumped clone_targets into the loading debug log

┌ Debug: Rejecting cache file /tmp/jl_YlNZqW/compiled/v1.10/Zlib_jll/xjq3Q_pQJwP.ji for Zlib_jll [83775a58-1f1d-513f-b197-d71354ab007a] since pkgimage can't be loaded on this target
│   clone_targets = UInt8[0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0b, 0x00, 0x00, 0x00, 0x03, 0x32, 0xd8, 0x36, 0x00, 0x00, 0x00, 0x00, 0x29, 0x01, 0x88, 0x21, 0x90, 0x06, 0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x61, 0x01, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x01, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x27, 0xdc, 0x63, 0x59, 0x00, 0x3a, 0x00, 0x41, 0x45, 0x03, 0x00, 0x08, 0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x30, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x7a, 0x6e, 0x76, 0x65, 0x72, 0x33, 0x00, 0x00, 0x00, 0x00]
└ @ Base loading.jl:2564

cc. @vchuravy

@vchuravy

vchuravy commented Jan 8, 2023

Copy link
Copy Markdown
Member

What are you setting JULIA_CPU_TARGET to?

@IanButterworth

Copy link
Copy Markdown
Member Author

During tests it appears to be the default

Sys.ARCH === :x86_64 ? "generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)" :

Comment thread src/PackageCompiler.jl Outdated
@IanButterworth

Copy link
Copy Markdown
Member Author

pkgimages are somehow disabled in ensurecompiled... ??

┌ Debug: ensurecompiled
│   isdefined(Base, :Linking) = true
│   (Base.JLOptions()).use_pkgimages = 0
└ @ PackageCompiler ~/work/PackageCompiler.jl/PackageCompiler.jl/src/PackageCompiler.jl:250

@IanButterworth

IanButterworth commented Jan 9, 2023

Copy link
Copy Markdown
Member Author

Ohhh.. julia-runtest is running with coverage, so the tests are being run without pkgimages. I guess we should maintain that throughout all the subprocesses

@IanButterworth

IanButterworth commented Jan 9, 2023

Copy link
Copy Markdown
Member Author

Ok, with coverage disabled (and thus pkgimages enabled and with -O3 set during precompilation) this is now hitting the issue I see locally

┌ Debug: Rejecting cache file /tmp/jl_zmpJYV/compiled/v1.9/LibSSH2_jll/K6mup_XSlU0.ji for LibSSH2_jll [29816b5a-b9ab-546f-933c-edad1886dfa8] since pkgimage can't be loaded on this target
└ @ Base loading.jl:2538

@IanButterworth
IanButterworth force-pushed the ib/outputo_fix branch 2 times, most recently from 984cdcd to b02c438 Compare January 9, 2023 03:47
@vchuravy

vchuravy commented Jan 9, 2023

Copy link
Copy Markdown
Member

Ok, with coverage disabled (and thus pkgimages enabled and with -O3 set during precompilation) this is now hitting the issue I see locally

┌ Debug: Rejecting cache file /tmp/jl_zmpJYV/compiled/v1.9/LibSSH2_jll/K6mup_XSlU0.ji for LibSSH2_jll [29816b5a-b9ab-546f-933c-edad1886dfa8] since pkgimage can't be loaded on this target
└ @ Base loading.jl:2538

I am not sure I understand the issue here. Is the issue that we have a time-out since we are rejecting every cache file?

What is happening is that we create a pkgimage cachefile for a particular target (either native or multiversioning) and then have started a Julia session that uses a different target.
timholy/PkgCacheInspector.jl#3 and https://docs.julialang.org/en/v1.10-dev/devdocs/pkgimg/#Package-images-optimized-for-multiple-microarchitectures

@vchuravy

Copy link
Copy Markdown
Member

Maybe the right answer here is to run with --pkgimage=no it seems not to make sense to do all the extra work in the context of PackageCompiler.jl

Also I think you need to set the environment flag not the command line flag since Julia will complain.

@IanButterworth

Copy link
Copy Markdown
Member Author

Maybe the right answer here is to run with --pkgimage=no

Yeah. Especially given the sysimage will be built with -O3 and most people's existing pkgimage caches will be -O2.

@KristofferC

Copy link
Copy Markdown
Member

If there is no chance to reuse the already precompiled files that a user has from an earlier Pkg.precompile then using pkg images seems pointless, yes.

@vchuravy

Copy link
Copy Markdown
Member

I mean PackageCompiler will not re-use that information at all. It would be a big rewrite to have PackageCompiler turn into a linker of package images.

@KristofferC

Copy link
Copy Markdown
Member

This is just for package load times. When you load a package into the process that outputs the sysimage, it will use a precompile file if such a file exist to speed up loading.

@IanButterworth

Copy link
Copy Markdown
Member Author

Note if it doesn't find a usable cache it currently hits the bug where myrand is zero.

@KristofferC

Copy link
Copy Markdown
Member

Yew, because __init__ functions do not run in the sysimage process which means the RNG is not seeded. That is what that test is testing, that we load from a ji file (and thus the earlier Pkg.precompile was successful).

@IanButterworth

Copy link
Copy Markdown
Member Author

Also I think you need to set the environment flag not the command line flag since Julia will complain.

@vchuravy I couldn't find the environment flag (assuming the sysimage step will fail currently because of this)

@IanButterworth

Copy link
Copy Markdown
Member Author

I'm surprised that 1.9 is passing but nightly is failing

@IanButterworth

Copy link
Copy Markdown
Member Author

I'm keen to get this fixed because I can't fully test 1.9 in our stack before PackageCompiler works.

Given 1.9-nightly works here, I could try this branch, but I'd prefer to use a release.

@IanButterworth IanButterworth changed the title Fix 1.9+ Fix 1.9 Jan 12, 2023
@IanButterworth

Copy link
Copy Markdown
Member Author

@vchuravy is the last commit what you meant on slack? And if so, is it backwards compatible, or do I need to only do this on 1.9+?

@vchuravy

Copy link
Copy Markdown
Member

Should be backwards compatible. Everyone else just ignores it.

@IanButterworth

Copy link
Copy Markdown
Member Author

hmm, then removing --cpu-target isn't backwards compatible. I'll push a fix

Comment thread src/PackageCompiler.jl
@IanButterworth

Copy link
Copy Markdown
Member Author

The handling here feels a bit messy, but as far as I understand it's now correct.
The failures on nightly appear to require c-level changes.
Perhaps a tidyup/refactoring could follow when nightly is fixed.

So I'll merge and I think a patch release should be made

@IanButterworth
IanButterworth merged commit 61a6f40 into JuliaLang:master Jan 13, 2023
@IanButterworth
IanButterworth deleted the ib/outputo_fix branch January 13, 2023 16:05
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.

RNG not being seeded during precompilation

4 participants