Skip to content

Adjust IProfiler locked entry assert - #20375

Merged
mpirvu merged 1 commit into
eclipse-openj9:masterfrom
cjjdespres:adjust-iprofiler-assert
Oct 25, 2024
Merged

Adjust IProfiler locked entry assert#20375
mpirvu merged 1 commit into
eclipse-openj9:masterfrom
cjjdespres:adjust-iprofiler-assert

Conversation

@cjjdespres

Copy link
Copy Markdown
Contributor

The TR_IProfiler::releaseAllEntries() function is used with DEBUG to check that all entries in the IProfiler _bcHashTable are unlocked at JVM exit. This function now compensates for a known race condition in TR_IProfiler::findOrCreateEntry() by also reporting the number of locked entries it encounters that were likely not a result of that race condition, so it can be asserted instead that that total is zero.

Fixes: #20344

@cjjdespres
cjjdespres requested a review from dsouzai as a code owner October 17, 2024 16:41
@cjjdespres

Copy link
Copy Markdown
Contributor Author

Attn @mpirvu. I believe this addresses the scenario I outlined in #20344 (comment), but it's very difficult to test.

@mpirvu mpirvu self-assigned this Oct 19, 2024

@mpirvu mpirvu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Just a small suggestion for a comment.

Comment thread runtime/compiler/runtime/IProfiler.hpp Outdated

TR_IPMethodHashTableEntry *findOrCreateMethodEntry(J9Method *, J9Method *, bool addIt, uint32_t pcIndex = ~0);
uint32_t releaseAllEntries();
// Returns the number of entries released, and also adds the number of

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: "// Returns the number of entries released, and also stores the number of"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

The TR_IProfiler::releaseAllEntries() function is used with DEBUG to
check that all entries in the IProfiler _bcHashTable are unlocked at JVM
exit. This function now compensates for a known race condition in
TR_IProfiler::findOrCreateEntry() by also reporting the number of locked
entries it encounters that were likely not a result of that race
condition, so it can be asserted instead that that total is zero.

Signed-off-by: Christian Despres <despresc@ibm.com>
@cjjdespres
cjjdespres force-pushed the adjust-iprofiler-assert branch from f5b11d3 to 134ccd9 Compare October 24, 2024 14:03
@mpirvu

mpirvu commented Oct 24, 2024

Copy link
Copy Markdown
Contributor

jenkins test sanity xlinuxjit,zlinuxjit,alinux64jit jdk21

@mpirvu

mpirvu commented Oct 25, 2024

Copy link
Copy Markdown
Contributor

zlinuxjit encountered a timeout in jdk_lang_0

@mpirvu

mpirvu commented Oct 25, 2024

Copy link
Copy Markdown
Contributor

Since the new code only affects debug builds, it could not have caused the issue seen in testing. Hence, merging.

@mpirvu
mpirvu merged commit e2d824b into eclipse-openj9:master Oct 25, 2024
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.

IProfiler assertion failure while building JDK with compiler in debug mode: some entries were still locked on shutdown

2 participants