8276095: ciReplay: replay failure due to incomplete ciMethodData information

[dean-long]

The replay data was missing MethodData::_invocation_counter. Adding it seems to fix the problem. @rwestrel please verify if it works for you. Also, with this change:

1. added a version number to the replay file
2. removed unnused ci fields
3. corrected comment in TestLambdas.java

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8276095: ciReplay: replay failure due to incomplete ciMethodData information

Reviewers

• Christian Hagedorn (@chhagedorn - Reviewer)
• Vladimir Kozlov (@vnkozlov - Reviewer) ⚠️ Review applies to 20bea84

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/6344/head:pull/6344
$ git checkout pull/6344

Update a local copy of the PR:
$ git checkout pull/6344
$ git pull https://git.openjdk.java.net/jdk pull/6344/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 6344

View PR using the GUI difftool:
$ git pr show -t 6344

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/6344.diff

[bridgekeeper]

👋 Welcome back dlong! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@dean-long The following label will be automatically applied to this pull request:

• hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 03: Full - Incremental (0552e47a)
• 02: Full - Incremental (46fd3fac)
• 01: Full - Incremental (20bea849)
• 00: Full (a7580022)

[chhagedorn]

Looks good to me! Thanks for also adapting the changes from 8275868 to use the new version number.

[openjdk]

@dean-long This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8276095: ciReplay: replay failure due to incomplete ciMethodData information

Reviewed-by: chagedorn, kvn

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 33 new commits pushed to the master branch:

• 02f7900: 8276932: G1: Annotate methods with override explicitly in g1CollectedHeap.hpp
• fdcd16a: 8277048: Tiny improvements to the specification text for java.util.Properties.load
• b231f5b: 8276921: G1: Remove redundant failed evacuation regions calculation in RemoveSelfForwardPtrHRClosure
• ca2efb7: 8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend
• 296780c: 8276983: Small fixes to DumpAllocStat::print_stats
• 8c5f030: 8276453: Undefined behavior in C1 LIR_OprDesc causes SEGV in fastdebug build
• 176d21d: 8276824: refactor Thread::is_JavaThread_protected
• 74f3e69: 8277071: [BACKOUT] JDK-8276743 Make openjdk build Zip Archive generation "reproducible"
• b85500e: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
• 0d2980c: 8258192: Obsolete the CriticalJNINatives flag
• ... and 23 more: https://git.openjdk.java.net/jdk/compare/ad3be04d2ac84836e393d696ff03ddfe72779094...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[vnkozlov]

okay

[dean-long]

I guess we could leave this in for old replay files with the initialization further down in ciReplay::initialize() if _version < 1.

Yes, it's necessary to parse the value for old replay files, but the value is never used. I'm not sure what you are suggesting about the initialization further down.

[dean-long]

However, thinking again about this, it should not happen that we parse a version number that's not supported

A user could be using an older JDK but accidentally try a newer replay file. That was the scenario I had in mind.

[chhagedorn]

I guess we could leave this in for old replay files with the initialization further down in ciReplay::initialize() if _version < 1.

Yes, it's necessary to parse the value for old replay files, but the value is never used. I'm not sure what you are suggesting about the initialization further down.

Ok, I was missing that the parsed value has no effect anyways. Then you do not need this code for the initialization on L1416 for _version < 1:

m->_current_mileage = rec->_current_mileage;

However, thinking again about this, it should not happen that we parse a version number that's not supported

A user could be using an older JDK but accidentally try a newer replay file. That was the scenario I had in mind.

That's a valid point for emitting a warning instead.

Changes look good to me!

[dean-long]

Thanks @chhagedorn and @vnkozlov.

[dean-long]

/integrate

[openjdk]

Going to push as commit 9326eb1.
Since your change was applied there have been 40 commits pushed to the master branch:

• 7a87041: 8275385: Change nested classes in jdk.jdi to static nested classes
• db0c8d5: 8274232: Cleanup unnecessary null comparison before instanceof check in jdk.jdi
• 1830b8d: 8275056: Virtualize G1CardSet containers over heap region
• fe45835: 8274856: Failing jpackage tests with fastdebug/release build
• 9046077: 8276084: Linux DEB Bundler: release number in outputted .deb file should be optional
• 7fc344d: 8277028: Use service type documentation as fallback for @provides
• 35a831d: 8272170: Missing memory barrier when checking active state for regions
• 02f7900: 8276932: G1: Annotate methods with override explicitly in g1CollectedHeap.hpp
• fdcd16a: 8277048: Tiny improvements to the specification text for java.util.Properties.load
• b231f5b: 8276921: G1: Remove redundant failed evacuation regions calculation in RemoveSelfForwardPtrHRClosure
• ... and 30 more: https://git.openjdk.java.net/jdk/compare/ad3be04d2ac84836e393d696ff03ddfe72779094...master

Your commit was automatically rebased without conflicts.

[openjdk]

@dean-long Pushed as commit 9326eb1.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.