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

Investigate replacing the foreign type GAP_jll.GapObj by struct GapObj x::Any end #817

Open
fingolfin opened this issue Jun 29, 2022 · 2 comments · May be fixed by gap-system/gap#4937
Open

Comments

@fingolfin
Copy link
Member

This may be able to avoid oscar-system/Oscar.jl#1400 and JuliaLang/PackageCompiler.jl#700

But I am not sure whether it (a) is possible at all, and (b) will resolve the issue. Of course if it was possible, that'd be nice in any case....

To test this theory, I need to

  1. create a patch for the GAP kernel that allows to pass in the GapObj type (bonus points if it fallbacks to work as it does right now).
    • idea for this: check if the "parent module" we pass to GAP_InitJuliaMemoryInterface already contains a global GapObj which is a struct type, and if so, use that
  2. adapt GAP.jl and GAP_jll to support that; this may even involve moving the early GAP initialization back from GAP_jll into GAP.jl (no need to do it in the JLL anymore, as the one reason for doing that was to have the type GapObj available when precompiling GAP.jl)
  3. ...
@fingolfin
Copy link
Member Author

I experimented with this, and found a first major hitch: I tried to do

mutable struct GapObj
  x::Any
end

but that doesn't work, because the pointer in a GAP master pointer does not actually point at the object, it points inside the object... I see potential ways to deal with that:

  1. use something like
mutable struct GapObj
  bag::Ptr{Cvoid}  # points inside the content bag
  realBag::Any     # the actual content bag, for the Julia GC
end

This way, code in the GAP C kernel which wants to access the content by double-dereferencing a bag pointer still can do it, while Julia can properly track the object relationships.

  1. rework some code in the GAP kernel to allow for the new layout; in theory that should only require changing a few helper functions (BAG_HEADER, CONST_BAG_HEADER, PTR_BAG, CONST_PTR_BAG and SET_PTR_BAG)

Approach 2 would be my preferred one. I'll experiment more

@fingolfin
Copy link
Member Author

I tried approach 2 back in July and got further, but then run into more crashes I couldn't figure out. Shelved it back then (but should have opened at least a PR with the code I had so far... sigh).

Anyway, today while debugging some other code, it hit me: we have a function ADDR_OBJ in GAP.jl and I forgot to adjust that one 🤦 so yeah it couldn't have worked.

I'll try if that helps ASAP, but didn't want to forget about it, hence this comment.

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 a pull request may close this issue.

1 participant