Skip to content

Do root scanning in scan_vm_specific_roots#24

Merged
wks merged 1 commit into
ruby:mainfrom
wks:fix/no-scan-roots-in-mutator-thread
Apr 8, 2025
Merged

Do root scanning in scan_vm_specific_roots#24
wks merged 1 commit into
ruby:mainfrom
wks:fix/no-scan-roots-in-mutator-thread

Conversation

@wks
Copy link
Copy Markdown
Collaborator

@wks wks commented Apr 7, 2025

We rely on scan_vm_specific_roots to reach all stacks via the following path:

VM -> ractors -> threads -> fibers -> stacks

@wks
Copy link
Copy Markdown
Collaborator Author

wks commented Apr 7, 2025

Currently make test-all still fails when running in parallel. See: #23 But it is probably unrelated to this change because it is still reproducible in the main branch.

I didn't remove the workaround objspace::vm_context. A full fix requires a change in the https://github.com/ruby/ruby repo. See #22

@wks wks changed the title Do nothing in scan_roots_in_mutator_thread Move Mutator struct to Ractor cache Apr 8, 2025
@wks wks force-pushed the fix/no-scan-roots-in-mutator-thread branch from a8be598 to ea97454 Compare April 8, 2025 01:41
@wks wks changed the title Move Mutator struct to Ractor cache Do root scanning in scan_vm_specific_roots Apr 8, 2025
We rely on scan_vm_specific_roots to reach all stacks via the following
path:

    VM -> ractors -> threads -> fibers -> stacks
@wks wks force-pushed the fix/no-scan-roots-in-mutator-thread branch from ea97454 to 0a6a835 Compare April 8, 2025 01:43
@wks wks marked this pull request as ready for review April 8, 2025 01:43
@wks wks merged commit 5139811 into ruby:main Apr 8, 2025
8 of 10 checks passed
@wks
Copy link
Copy Markdown
Collaborator Author

wks commented Apr 8, 2025

Some run test failures still occur when running with MarkSweep, with some test cases failing with timeouts and others with assertion errors. Since they are reproducible in the main branch, I think they are unlikely related to this PR, and I merged this PR anyway.

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.

2 participants