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

Use RSSReport class #19563

Merged
merged 1 commit into from
Jul 16, 2024
Merged

Use RSSReport class #19563

merged 1 commit into from
Jul 16, 2024

Conversation

gita-omr
Copy link
Contributor

@gita-omr gita-omr commented May 28, 2024

No description provided.

@dsouzai
Copy link
Contributor

dsouzai commented May 29, 2024

I'm not sure it makes sense for this to go into 0.46 since this seems to be a non functional (i.e., reporting) feature rather than a bug fix, esp since the depending OMR change likely needs some non-trivial changes. Is there a strong reason to get this into 0.46?

@gita-omr
Copy link
Contributor Author

I'm not sure it makes sense for this to go into 0.46 since this seems to be a non functional (i.e., reporting) feature rather than a bug fix, esp since the depending OMR change likely needs some non-trivial changes. Is there a strong reason to get this into 0.46?

I would say it does add a new feature (under an option). We planned to get it into 0.46 - just were a bit late.

@pshipton
Copy link
Member

pshipton commented May 31, 2024

FYI there are conflicts that need to be resolved before this is merged.

I don't have any objection to putting this in 0.46 as long as this is merged to master and there are no resulting problems. My understanding is it's disabled by default, we haven't started any Milestone builds for 0.46 yet, and we're well in advance of M2. If we are putting it in, it would be nice to have it in before we start M1 builds next week.

@gita-omr
Copy link
Contributor Author

gita-omr commented Jun 2, 2024

Addressed OMR comments and rebased.

runtime/compiler/runtime/DataCache.hpp Outdated Show resolved Hide resolved
runtime/compiler/control/rossa.cpp Show resolved Hide resolved
runtime/compiler/build/files/common.mk Outdated Show resolved Hide resolved
runtime/compiler/runtime/DataCache.cpp Outdated Show resolved Hide resolved
runtime/compiler/runtime/DataCache.cpp Show resolved Hide resolved
runtime/compiler/runtime/DataCache.cpp Outdated Show resolved Hide resolved
runtime/compiler/runtime/J9CodeCache.hpp Outdated Show resolved Hide resolved
runtime/compiler/runtime/J9CodeCache.cpp Outdated Show resolved Hide resolved
@gita-omr gita-omr changed the title Use RSSReport class WIP: Use RSSReport class Jun 10, 2024
@gita-omr
Copy link
Contributor Author

Thanks for the review. I will address it shortly. I decided to make this PR WIP and include ability to use this class from VM as well.

@gita-omr
Copy link
Contributor Author

Rebased.

@gita-omr
Copy link
Contributor Author

I think I addressed all the comments and I am not planning on adding any new interfaces to that class right now. @dsouzai @mpirvu could you please take another look?

@gita-omr gita-omr changed the title WIP: Use RSSReport class Use RSSReport class Jun 20, 2024
runtime/compiler/runtime/DataCache.cpp Outdated Show resolved Hide resolved
runtime/oti/j9nonbuilder.h Outdated Show resolved Hide resolved
runtime/compiler/runtime/J9CodeCache.cpp Outdated Show resolved Hide resolved
@gita-omr
Copy link
Contributor Author

gita-omr commented Jul 3, 2024

Track the whole Code Cache vs just cold part.

@gita-omr
Copy link
Contributor Author

gita-omr commented Jul 3, 2024

Addressed latest comments

@gita-omr
Copy link
Contributor Author

gita-omr commented Jul 9, 2024

Rebased

@gita-omr
Copy link
Contributor Author

gita-omr commented Jul 9, 2024

Added check for rssReport != NULL inside jitAddNewLowToHighRSSRegion().

@gita-omr
Copy link
Contributor Author

gita-omr commented Jul 9, 2024

Addressed latest comments

@mpirvu
Copy link
Contributor

mpirvu commented Jul 10, 2024

The commits should be squashed into one.

@gita-omr
Copy link
Contributor Author

Squashed all the commits.

@dsouzai
Copy link
Contributor

dsouzai commented Jul 11, 2024

jenkins test sanity.functional all jdk21

@dsouzai
Copy link
Contributor

dsouzai commented Jul 11, 2024

@gita-omr the builds are failing due to:

[2024-07-11T14:05:10.969Z] In file included from /home/jenkins/workspace/Build_JDK21_x86-64_linux_Personal/openj9/runtime/compiler/control/CompilationThread.hpp:38,
[2024-07-11T14:05:10.969Z]                  from /home/jenkins/workspace/Build_JDK21_x86-64_linux_Personal/openj9/runtime/compiler/net/ServerStream.hpp:29,
[2024-07-11T14:05:10.969Z]                  from /home/jenkins/workspace/Build_JDK21_x86-64_linux_Personal/openj9/runtime/compiler/control/CompilationRuntime.hpp:45,
[2024-07-11T14:05:10.969Z]                  from /home/jenkins/workspace/Build_JDK21_x86-64_linux_Personal/openj9/runtime/compiler/../compiler/codegen/J9CodeGenerator.hpp:45,
[2024-07-11T14:05:10.969Z]                  from /home/jenkins/workspace/Build_JDK21_x86-64_linux_Personal/openj9/runtime/compiler/x/codegen/J9CodeGenerator.hpp:26,
[2024-07-11T14:05:10.969Z]                  from /home/jenkins/workspace/Build_JDK21_x86-64_linux_Personal/openj9/runtime/compiler/x/amd64/codegen/J9CodeGenerator.hpp:39,
[2024-07-11T14:05:10.969Z]                  from /home/jenkins/workspace/Build_JDK21_x86-64_linux_Personal/openj9/runtime/compiler/codegen/CodeGenGC.cpp:34:
[2024-07-11T14:05:10.970Z] /home/jenkins/workspace/Build_JDK21_x86-64_linux_Personal/openj9/runtime/compiler/runtime/DataCache.hpp:29:10: fatal error: runtime/OMRRSSReport.hpp: No such file or directory
[2024-07-11T14:05:10.970Z]    29 | #include "runtime/OMRRSSReport.hpp"
[2024-07-11T14:05:10.970Z]       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
[2024-07-11T14:05:10.970Z] compilation terminated.

@dsouzai
Copy link
Contributor

dsouzai commented Jul 11, 2024

This PR will conflict with #19850 once that's been merged.

@gita-omr
Copy link
Contributor Author

Rebased and removed dependency from the description.

@gita-omr
Copy link
Contributor Author

Not sure why OMRRSSReport.hpp was not found. Maybe eclipse-omr/omr#7349 was not picked up by the build yet.

@dsouzai
Copy link
Contributor

dsouzai commented Jul 12, 2024

jenkins compile xlinux jdk21

@dsouzai
Copy link
Contributor

dsouzai commented Jul 12, 2024

jenkins test sanity.functional all jdk21

@gita-omr
Copy link
Contributor Author

The windows build failure seems to be unrelated.

@dsouzai
Copy link
Contributor

dsouzai commented Jul 16, 2024

The windows build failure seems to be unrelated.

Windows machines are all down right now. Everything else passed so will merge.

@dsouzai dsouzai merged commit c9ac00e into eclipse-openj9:master Jul 16, 2024
14 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants