Skip to content

julia_gc: include julia.h globally for better CHANGED_BAG - #6490

Draft
fingolfin wants to merge 2 commits into
masterfrom
mh/julia-gc-global-header
Draft

julia_gc: include julia.h globally for better CHANGED_BAG#6490
fingolfin wants to merge 2 commits into
masterfrom
mh/julia-gc-global-header

Conversation

@fingolfin

@fingolfin fingolfin commented Aug 5, 2026

Copy link
Copy Markdown
Member

CHANGED_BAG is supposed to have minimal overhead, which normally is achieved by inlining it. But we never had this for the Julia GC, mainly because it was painful to do so in our C++ source files. This PR addresses this.

As a side effect, this enables future work I have planned that will need the Julia headers globally.

Note that CHANGED_BAG cannot simply call jl_gc_wb_back: Julia declares that as static inline, and C forbids referencing an identifier with internal linkage from an inline function with external linkage. Marking CHANGED_BAG as static inline instead is not an option either, as it is used by other EXPORT_INLINE functions (such as PushPlist), which then run into the very same problem. So we inline a copy of Julia's write barrier, which only refers to jl_gc_queue_root.

Since src/gasman.h now includes julia.h, all GAP kernel extensions need the Julia headers, too, so we pass those on to packages via sysinfo.gap. The JULIA=yes CI job now also runs testmockpkg to catch regressions here.

This also means that macros defined by julia.h can now clash with code in kernel extensions. One such clash affects GAP.jl's JuliaInterface, which declares jl_n_threads itself; the CI with GAP.jl jobs will therefore keep failing until oscar-system/GAP.jl#1412 has been merged.

CHANGED_BAG is supposed to have minimal overhead, which normally
is achieved by inlining it. But we never had this for the Julia
GC, mainly because it was painful to do so in our C++ source
files. This PR addresses this.

As a side effect, this enables future work I have planned that
will need the Julia headers globally.
@fingolfin
fingolfin requested a review from lgoettgens August 5, 2026 16:29
@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: build system topic: kernel release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: julia Julia GC integration and related matters labels Aug 5, 2026
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 79.03%. Comparing base (c0ec89a) to head (674a54f).

Files with missing lines Patch % Lines
src/gasman.h 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6490   +/-   ##
=======================================
  Coverage   79.03%   79.03%           
=======================================
  Files         685      685           
  Lines      293790   293793    +3     
  Branches     8664     8642   -22     
=======================================
+ Hits       232203   232206    +3     
  Misses      59786    59786           
  Partials     1801     1801           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fingolfin
fingolfin marked this pull request as draft August 5, 2026 21:23
Inlining `CHANGED_BAG` by calling `jl_gc_wb_back` does not work: Julia
declares that function `static inline`, and C forbids referencing an
identifier with internal linkage from an inline function with external
linkage. Making `CHANGED_BAG` itself `static inline` is not an option
either, as it is used by other `EXPORT_INLINE` functions, which then run
into the very same problem; and turning all of those into `static inline`
would drop symbols from libgap which GAP.jl relies on. So instead inline
a copy of Julia's write barrier, which only refers to `jl_gc_queue_root`.

Since src/gasman.h now includes julia.h, packages need the Julia headers
as well, so pass them on via sysinfo.gap. The flags reported by
julia-config.jl are quoted, which does not survive the way sysinfo.gap is
consumed, hence strip the quotes; and resolve the clash between Julia's
`FORCE_INLINE` and the one used by our copy of MurmurHash3.

AI disclosure: Claude Code (Opus 5) diagnosed the build failures, drafted
the changes, and verified them locally.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@fingolfin

Copy link
Copy Markdown
Member Author

This may need some more refining...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: build system topic: julia Julia GC integration and related matters topic: kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant