Skip to content

Conversation

@no-defun-allowed
Copy link
Contributor

If an nmethod is in the mature roots and the nmethod is re-registered due to code patching, the nmethod will appear in both the nursery roots and the mature roots for one collection, causing some roots to be scanned twice. This PR removes an nmethod from the mature roots, when the nmethod is re-registered, so that the nmethod will only be present in the nursery.

@k-sareen
Copy link
Collaborator

k-sareen commented Dec 1, 2025

Closes #331

@wks
Copy link
Contributor

wks commented Dec 1, 2025

Could you re-open this PR to target the jdk-11 branch, instead? You can open another PR for jdk-21 if you want to.

(Edit: It turned out that there is a way to set the base branch without re-opening the PR. I have made the change already.)

@wks wks changed the base branch from master to jdk-11 December 1, 2025 07:02
Copy link
Contributor

@wks wks left a comment

Choose a reason for hiding this comment

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

LGTM

Let's merge this obvious fix for now, and I'll refactor the mutex later, as I mentioned in the other comment.

NMETHOD_SLOTS.with_borrow_mut(|slots| {
if !slots.is_empty() {
let mut roots = crate::NURSERY_CODE_CACHE_ROOTS.lock().unwrap();
let mut mature_roots = crate::MATURE_CODE_CACHE_ROOTS.lock().unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Holding two locks at the same time is prone to dead-locking, especially when ScanCodeCacheRoots::do_work acquires the two locks in the opposite order. One way to fix it is putting the two hash maps under the same lock. I refactored this part of code locally when trying to address the "fix relocation" thing, but haven't merged it yet because it is somewhat unrelated to the "fix relocation" itself. I'll make another pull request later for the refactoring.

@wks wks merged commit 3e27334 into mmtk:jdk-11 Dec 2, 2025
56 checks passed
wks pushed a commit to wks/mmtk-openjdk that referenced this pull request Dec 3, 2025
If an nmethod is in the mature roots and the nmethod is re-registered
due to code patching, the nmethod will appear in both the nursery roots
and the mature roots for one collection, causing some roots to be
scanned twice. This PR removes an nmethod from the mature roots, when
the nmethod is re-registered, so that the nmethod will only be present
in the nursery.
@wks wks mentioned this pull request Dec 3, 2025
wks added a commit that referenced this pull request Dec 3, 2025
If an nmethod is in the mature roots and the nmethod is re-registered
due to code patching, the nmethod will appear in both the nursery roots
and the mature roots for one collection, causing some roots to be
scanned twice. This PR removes an nmethod from the mature roots, when
the nmethod is re-registered, so that the nmethod will only be present
in the nursery.

This PR is ported from #336 for
the `jdk-21` branch.

Co-authored-by: Hayley Patton <[email protected]>
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