Open mjrodgers opened 6 months ago
ThomasBreuer
commented
6 months ago I cannot reproduce this problem under Ubuntu 20.04 with Julia 1.8.5 and 1.9.0, with various values for the -t command line option. Would it make sense to try other Julia versions with this setup?
mjrodgers
commented
6 months ago I have it working perfectly fine with 1.9.3, it seems to be an issue only with julia 1.10
On Jan 19, 2024, at 12:59 PM, Thomas Breuer @.***> wrote:
I cannot reproduce this problem under Ubuntu 20.04 with Julia 1.8.5 and 1.9.0, with various values for the -t command line option. Would it make sense to try other Julia versions with this setup?
— Reply to this email directly, view it on GitHub https://github.com/oscar-system/GAP.jl/issues/960#issuecomment-1900285584, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANWIWO5Z2YNFV6UXNMWSKDYPJNYXAVCNFSM6AAAAABCBUNQWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBQGI4DKNJYGQ. You are receiving this because you authored the thread.
benlorenz
commented
6 months ago A workaround to load GAP in a julia session with threads enabled is to disable gcthreads with --gcthreads=1 (this corresponds to how the GC works in 1.9).
GC threads are new in julia 1.10 and enabled by default when threads are enabled, and the number of gcthreads is half the number of compute threads. This explains why the problems starts with 4 threads.
The following seems to work fine with julia 1.10:
$ julia-1.10.0 --project=. -t 4 --gcthreads=1
_
_ _ _(_)_ | Documentation: https://docs.julialang.org
(_) | (_) (_) |
_ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 1.10.0 (2023-12-25)
_/ |\__'_|_|_|\__'_| | Official https://julialang.org/ release
|__/ |
julia> using GAP
┌───────┐ GAP 4.12.2 of 2022-12-18
│ GAP │ https://www.gap-system.org
└───────┘ Architecture: x86_64-pc-linux-gnu-julia1.10-64-kv8
Configuration: gmp 6.2.1, Julia GC, Julia 1.10.0, readline
Loading the library and packages ...
Packages: AClib 1.3.2, Alnuth 3.2.1, AtlasRep 2.1.6, AutPGrp 1.11, Browse 1.8.21, CRISP 1.4.6,
Cryst 4.1.25, CrystCat 1.1.10, CTblLib 1.3.4, FactInt 1.6.3, FGA 1.4.0, GAPDoc 1.6.6,
IRREDSOL 1.4.4, JuliaInterface 0.10.1, LAGUNA 3.9.5, Polenta 1.3.10, Polycyclic 2.16,
PrimGrp 3.4.3, RadiRoot 2.9, ResClasses 4.7.3, SmallGrp 1.5.1, Sophus 1.27, SpinSym 1.5.2,
TomLib 1.2.9, TransGrp 3.6.3, utils 0.81
Try '??help' for help. See also '?copyright', '?cite' and '?authors'Tests also work in this configuration: Pkg.test("GAP", julia_args=["-t 4", "--gcthreads=1"]).
fingolfin
commented
6 months ago Thanks for writing up a report, @mjrodgers I can reproduce it with Julia 1.10.0 on macOS. I will look into this, but possibly only after February 1st, due to the book
fingolfin
commented
4 months ago There are multiple issues here in the GAP-Julia GC integration. One is the now incorrect usage of the variable JuliaTLS in the GAP kernel, which is based on the assumption that there is a single GC job. JuliaTLS is mostly an optimization, and all its uses could simply be replaced by a call to jl_get_ptls_states() -- and also most uses could be avoided by a change to the GAP kernel (basically MarkBag and all related functions would need an extra argument, a void * ref pointer, which is set to the tls pointer when using the Julia GC in GAP and otherwise ignored)
Then there is the use of a bunch of global variables in there (in GAP's src/julia_gc.c to be precise) which is not multi-thread safe, and needs some strategy for that (be it locks, or making them thread local, or ...)
MarkCache and friends: could probably be resolved by activating REQUIRE_PRECISE_MARKING which should be safe these days.RootTaskOfMainThread and GapStackBottom are (I think) not used for GAP.jl, only for the CI tests over at the GAP repositoryYoungRef: could be made thread local, but if we add a void * ref to MarkBag and friends anyway, then that could just point at a struct on the stack which contains Julias ptls and YoungRefDatatypeGapObj, DatatypeSmallBag, DatatypeLargeBag, IsJuliaMultiThreaded, MaxPoolObjSize, TabMarkFuncBags: only set once at the start and otherwise read-only, so hopefully are fineExtraMarkFuncBags unusedFullGC set at the start of each GC, I hope this is fine, but should have a closer lookStartTime, TotalTime should be fine (but even if not, nothing bad can happen if they are wrong)GlobalAddr, GlobalCookie, GlobalCount: should be fine... I thinktask_stacks requires closer inspection
simonbrandhorst
commented
3 months ago Just hit the same bug when installing Oscar and got a segmentation fault. Since this wil be a while until it is fixed, we could check the number of GC threads at startup and raise a useful error?
fingolfin
commented
2 months ago I made some progress on this today: turns out I have to change function ScanTaskStack in the GAP kernel from MarkFromList(jl_get_ptls_states(), stack); to MarkFromList(task->ptls, stack); -- apparently the jl_get_ptls_states is not safe to use in a GC thread.
I also added a mutex to protect access to task_stacks, and enabled REQUIRE_PRECISE_MARKING to completely eliminate a bunch of other global variables (which are problematic in multi threading). With all these changes using GAP works with multiple GC threads (in contrast to stock GAP.jl 0.11.0 using GAP 4.13.0, which immediately hard crashes, i.e., is worse thatn GAP.jl 0.10.0 with GAP 4.12.2).
Alas, there are still issues. So the next thing I did was to add a global GAP list and then any allocation is added to that list, and any GC marks that list -- this should prevent any GAP allocation from ever be garbage collected. (Clearly this is not useful for a production system, but it helps to exclude certain issues).
But despite this, I am still seeing errors that are highly suggestive of GAP objects being GCed prematurely. Hrm.
Edit (@thoma): As a workaround, start julia with
--gcthreads=1.When starting julia using multiple threads (
julia -t 4),using GAPcan fail. This also seems to leave julia in a fragile state, and I get a segfault when quitting.Interestingly,
using GAPseems to work fine if I launch julia using 3 threads, but 4 is a problem.I'm running on an Intel Mac running Sonoma 14.2.1, julia 1.10.0, GAP 0.10.1