8000 Remove TR::Options::INLINE_failedToDevirtualize usage by mpirvu · Pull Request #22008 · eclipse-openj9/openj9 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove TR::Options::INLINE_failedToDevirtualize usage #22008

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 3, 2025
Merged

Remove TR::Options::INLINE_failedToDevirtualize usage #22008

merged 1 commit into from
Jun 3, 2025

Conversation

mpirvu
Copy link
Contributor
@mpirvu mpirvu commented Jun 2, 2025

TR::Options::INLINE_failedToDevirtualize and TR::Options::INLINE_failedToDevirtualizeInterface are not really options, but rather statistics. The code that updates them (present in omr project) is supposed to be removed and thus we must remove their references in OpenJ9 as well.

@mpirvu
Remove TR::Options::INLINE_failedToDevirtualize usage
< 8000 div class="pr-1 flex-shrink-0" style="width: 16px;">
5226f3e 
`TR::Options::INLINE_failedToDevirtualize` and
`TR::Options::INLINE_failedToDevirtualizeInterface`
are not really options, but rather statistics.
The code that updates them (present in omr project)
is supposed to be removed and thus we must remove
their references in OpenJ9 as well.

Signed-off-by: Marius <mpirvu@ca.ibm.com>
@mpirvu mpirvu requested a review from dsouzai as a code owner June 2, 2025 21:38
@mpirvu mpirvu added the comp:jit label Jun 2, 2025
@mpirvu mpirvu mentioned this pull request Jun 2, 2025
Merged
@mpirvu
Copy link
Contributor Author
mpirvu commented Jun 3, 2025

jenkins compile all jdk17

@mpirvu
Copy link
Contributor Author
mpirvu commented Jun 3, 2025

Attn @vijaysun-omr This PR is ready to be reviewed/merged.

@vijaysun-omr
Copy link
Contributor

Just out of curiosity, are these stats not sensible ones to track ? i.e. the number of failed devirtualizations.

Maybe the problem is that the way that these values are being tracked is incorrect anyway (?)

@mpirvu
Copy link
Contributor Author
mpirvu commented Jun 3, 2025

Maybe the problem is that the way that these values are being tracked is incorrect anyway (?)

That's indeed the case. I don't understand the omr code that increments them (see eclipse-omr/omr#7773):

   if ( getUtil()->getCallCount(callNode) > 0)
      {
      if (callsite->isInterface())
         TR::Options::INLINE_failedToDevirtualize ++;
      else
         TR::Options::INLINE_failedToDevirtualizeInterface ++;
      }

@vijaysun-omr
Copy link
Contributor

Looks odd to me too. I can only imagine that it prints the "call count" for such virtual/interface calls thinking that it is a reflection of how many times those calls ran (as reported by the profiler).

@vijaysun-omr
Copy link
Contributor

In any case, we have never really used this in recent years to my knowledge (I did not even know it existed). So I am fine with this being removed, and if we really wanted it in future, we would implement it in a different / cleaner way.

@vijaysun-omr vijaysun-omr merged commit a702e12 into eclipse-openj9:master Jun 3, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dsouzai dsouzai dsouzai approved these changes

@vijaysun-omr vijaysun-omr vijaysun-omr approved these changes

Assignees
No one assigned
Labels
comp:jit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mpirvu @vijaysun-omr @dsouzai
0