Each GAP.jl session uses fresh dir for mutable GAP root

[fingolfin]

Resolves #993 -- I measured that it is not slower than the current approach which fully regenerates the GAP root each time anyway.

Specifically on my test machines it takes about 0.09 - 0.11 seconds. Which is not nothing, but still relatively little.

An alternative would be to keep using a scratch space but be more aggressive in switching to a new one when anything changes, i.e. incorporating a hash value into its name that involves more components, such as

• a hash of the content of setup.jl
• hashes of the artifact dirs of all involved artifacts / JLLs
• ...

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.78%. Comparing base (013ffbc) to head (88c52bd).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1079      +/-   ##
==========================================
+ Coverage   82.73%   82.78%   +0.04%     
==========================================
  Files          50       50              
  Lines        3823     3833      +10     
==========================================
+ Hits         3163     3173      +10     
  Misses        660      660              

Files with missing lines	Coverage Δ
src/GAP.jl	87.50% <100.00%> (+0.40%)	⬆️
src/setup.jl	88.79% <100.00%> (+0.61%)	⬆️

[lgoettgens]

Specifically on my test machines it takes about 0.09 - 0.11 seconds. Which is not nothing, but still relatively little.

I compared de7bed9 (tip of this PR) to 0412fa6 (the master commit this branch is based on) w.r.t. @time @time_imports using GAP (after doing ] up, waiting for precompilation and running multiple times to simulate a non-dev usage). There were no statistically significant changes between the two commits. This may be due to the removal of a pid-file usage that could counter the times you measured.

The only thing I am a little bit worried about is that some OSs may clear the /tmp directory after a set number of days. If this is done while a long running GAP.jl/OSCAR computation is still running, this may break.

[fingolfin]

Oh, good point about stuff in /tmp or /var/tmp potentially being deleted by cron jobs. That's indeed a thing.

So how about instead we keep the scratch space and create the temp dirs in it (mktempdir has an option for that). The scratch space name then could stay relatively fixed. The tempdir names could consist of the current date&time in ISO8601 format followed by a random string (I am suggesting to put in time because that way it's trivial to spot any such dirs that should have been deleted but were not e.g. due to a crash. It might also help recognize the dirs for the "long running" jobs).

[lgoettgens]

I don't understand what you would want to use mktempdir for? Shouldn't we let Scratch.jl create these directories using `get_scratch!`? This way, Scratch.jl would take care of garbage collecting old unused folders.

…

On December 11, 2024 9:27:45 PM GMT+01:00, Max Horn ***@***.***> wrote: Oh, good point about stuff in `/tmp` or `/var/tmp` potentially being deleted by cron jobs. That's indeed a thing. So how about instead we keep the scratch space and create the temp dirs in it (`mktempdir` has an option for that). The scratch space name then could stay relatively fixed. The tempdir names could consist of the current date&time in ISO8601 format followed by a random string (I am suggesting to put in time because that way it's trivial to spot any such dirs that should have been deleted but were not e.g. due to a crash. It might also help recognize the dirs for the "long running" jobs). -- Reply to this email directly or view it on GitHub: #1079 (comment) You are receiving this because you commented. Message ID: ***@***.***>

[fingolfin]

I want a different directory for each julia process loading GAP.jl. Scratchspaces alone do not provide for that.

[lgoettgens]

No, it does. If you call get_scratch!(some_unique_string_that_contains_datetime), this creates a unique folder (as the key is unique). However, Scratch.jl will track that usage dates of these folders and gc them after some time

[fingolfin]

@lgoettgens updated now as discussed on #1083

[lgoettgens]

Why did you drop the changes in https://github.com/oscar-system/GAP.jl/compare/de7bed9931a62d2bcd003442268379f59359d274..d8459a9b7730af2650c491fe0b1e709f6b756d1c#diff-68a57defa4eef4087b4707a47026264bd208020e98af8639ab658853d0f1f35d (packages.jl)? It seems to me that this could be related to the failures here

[fingolfin]

Nope the failure is caused by the use of our gap.sh wrapper, which now every time has a fresh GAP root. That messes with the build system of JuliaInterface: the configure script read sysinfo.gap and uses that to generate Makefile from Makefile.in. But also the path to the sysinfo.gap file is put into Makefile, so facilitate re-generation of everything if the GAP config changes. As a result, when we run make clean in etc/ci_test.sh it runs into an error because it can't fight sysinfo.gap.

Gotta think a bit about it...

[fingolfin]

This is now reduced to generating an additional GAP root dir containing a sysinfo.gap and a gac executable.

Perhaps time now to transform this:

• produce a function which takes a directory path and puts sysinfo.gap and gac (and whatever
• then only run this on a fresh temp dir just before invoking GAP.Packages.install (perhaps caching the resulting dir for the duration of the GAP.jl session)
• perhaps create_gap_sh could also make use of it, and produce a gac and sysinfo.gap which fit together with the produce gap shell script (towards resolving Invalid path to GAP executable in sysinfo #1105)
• actually with that last bit we could circle back to the top: before calling GAP.Packages.install perhaps we should call this enhance create_gap_sh and make sure PackageManger "sees" its sysinfo.gap (and of course gac, gap)

[fingolfin]

This would also hopefully allow us to deal with the three packages listed under "Missing Software: GAP" in #1122

[fingolfin]

The stuff I just mentioned probably also would resolve #840