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

Update JLLs for Julia nightly compat & GC fix #1031

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

fingolfin
Copy link
Member

The fix in the JLL is to disable task stack scanning. This unfortunately has a severe performance penalty, but at least we can now test if it really resolves the GC crashes in all cases while I work on a more permanent solution.

The fix in the JLL is to disable task stack scanning. This
unfortunately has a severe performance penalty, but at least
we can now test if it really resolves the GC crashes in all
cases while I work on a more permanent solution.
Copy link

codecov bot commented Sep 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.95%. Comparing base (b6cb993) to head (7a4a8be).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1031      +/-   ##
==========================================
- Coverage   76.82%   75.95%   -0.88%     
==========================================
  Files          51       51              
  Lines        4182     4175       -7     
==========================================
- Hits         3213     3171      -42     
- Misses        969     1004      +35     

see 3 files with indirect coverage changes

Copy link
Member

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have checked the effect of the change on my notebook (Ubuntu 20.04.1 LTS):
With this pull request, the GAP.jl test suite runs in Julia 1.10.4 without errors.
Without this pull request, Julia 1.10.4 crashes as described in a comment in #1029.

@fingolfin
Copy link
Member Author

Excellent. Unfortunately CI failed widely -- but it seems due to network issues (phew). I've restarted it now. If it passes I suggest we merge this for now. I'll work on a better fix, but this also contains fixes for Julia nightly so is useful anyway.

So if you agree, just approve this PR?

@fingolfin fingolfin enabled auto-merge (squash) September 4, 2024 08:01
Copy link
Member

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

@lgoettgens
Copy link
Member

The required CI jobs still wait for the 1.9 job, but this does not exist anymore due to #1022. The list of required passing jobs needs to be adapted to reflect the changes there.

@fingolfin fingolfin merged commit 71d71f8 into oscar-system:master Sep 4, 2024
16 of 21 checks passed
@fingolfin fingolfin deleted the mh/fixes branch September 4, 2024 11:36
@fingolfin
Copy link
Member Author

Unfortunately there are still failing jobs, but it was merged anyway because I only remove the 1.9 job from the branch protection rules, but had not yet add the new checks I wanted to add (doing that now).

Anyway: e.g. https://github.com/oscar-system/GAP.jl/actions/runs/10696442564/job/29654943194?pr=1031 fails.

So it seems there is still more to uncover -- unfortunately for now I have run out of test cases that reproduce the crash locally for me. I'll continue my search :-(

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.

3 participants