RFR: 8298341: Ensure heap growth in TestNativeMemoryUsageEvents.java
Please review this test fix. **Summary** The new test TestNativeMemoryUsageEvents.java sometimes fail. The reason is that the _NativeMemoryUsage_ and _NativeMemoryUsageTotal_ events want to share a single `NMTUsage` snapshot. To achieve this there is an `AgeThreshold` set to 50ms that is considered when sending the events. If the usage snapshot is new enough we don't refresh it, but reuse it. In the test this becomes problematic if the recording is shorter than 50ms, then both the beginChunk event and the endChunk event will use the same NMT data. This could be solved by using the feature currently reviewed in #11541 and this is the plan, see: [JDK-8298276](https://bugs.openjdk.org/browse/JDK-8298276). But since this is not yet done, I propose to fix the test for now by just adding a short sleep during the recording to ensure the endChunk event is delayed enough to cause a new NMT snapshot to be taken. **Testing** * Local testing with a larger `AgeThreshold` to see that a sleep really helps the problem. * Mach5 testing to verify the fix is good there as well, this is currently running. ------------- Commit messages: - 8298341: Ensure heap growth in TestNativeMemoryUsageEvents.java Changes: https://git.openjdk.org/jdk/pull/11579/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11579&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8298341 Stats: 9 lines in 1 file changed: 9 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/11579.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11579/head:pull/11579 PR: https://git.openjdk.org/jdk/pull/11579
On Thu, 8 Dec 2022 07:58:39 GMT, Stefan Johansson <sjohanss@openjdk.org> wrote:
Please review this test fix.
**Summary** The new test TestNativeMemoryUsageEvents.java sometimes fail. The reason is that the _NativeMemoryUsage_ and _NativeMemoryUsageTotal_ events want to share a single `NMTUsage` snapshot. To achieve this there is an `AgeThreshold` set to 50ms that is considered when sending the events. If the usage snapshot is new enough we don't refresh it, but reuse it. In the test this becomes problematic if the recording is shorter than 50ms, then both the beginChunk event and the endChunk event will use the same NMT data.
This could be solved by using the feature currently reviewed in #11541 and this is the plan, see: [JDK-8298276](https://bugs.openjdk.org/browse/JDK-8298276). But since this is not yet done, I propose to fix the test for now by just adding a short sleep during the recording to ensure the endChunk event is delayed enough to cause a new NMT snapshot to be taken.
**Testing** * Local testing with a larger `AgeThreshold` to see that a sleep really helps the problem. * Mach5 testing to verify the fix is good there as well, this is currently running.
Marked as reviewed by egahlin (Reviewer). ------------- PR: https://git.openjdk.org/jdk/pull/11579
On Thu, 8 Dec 2022 08:12:36 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Please review this test fix.
**Summary** The new test TestNativeMemoryUsageEvents.java sometimes fail. The reason is that the _NativeMemoryUsage_ and _NativeMemoryUsageTotal_ events want to share a single `NMTUsage` snapshot. To achieve this there is an `AgeThreshold` set to 50ms that is considered when sending the events. If the usage snapshot is new enough we don't refresh it, but reuse it. In the test this becomes problematic if the recording is shorter than 50ms, then both the beginChunk event and the endChunk event will use the same NMT data.
This could be solved by using the feature currently reviewed in #11541 and this is the plan, see: [JDK-8298276](https://bugs.openjdk.org/browse/JDK-8298276). But since this is not yet done, I propose to fix the test for now by just adding a short sleep during the recording to ensure the endChunk event is delayed enough to cause a new NMT snapshot to be taken.
**Testing** * Local testing with a larger `AgeThreshold` to see that a sleep really helps the problem. * Mach5 testing to verify the fix is good there as well, this is currently running.
Marked as reviewed by egahlin (Reviewer).
Thanks @egahlin and @tstuefe for the quick reviews. ------------- PR: https://git.openjdk.org/jdk/pull/11579
On Thu, 8 Dec 2022 07:58:39 GMT, Stefan Johansson <sjohanss@openjdk.org> wrote:
Please review this test fix.
**Summary** The new test TestNativeMemoryUsageEvents.java sometimes fail. The reason is that the _NativeMemoryUsage_ and _NativeMemoryUsageTotal_ events want to share a single `NMTUsage` snapshot. To achieve this there is an `AgeThreshold` set to 50ms that is considered when sending the events. If the usage snapshot is new enough we don't refresh it, but reuse it. In the test this becomes problematic if the recording is shorter than 50ms, then both the beginChunk event and the endChunk event will use the same NMT data.
This could be solved by using the feature currently reviewed in #11541 and this is the plan, see: [JDK-8298276](https://bugs.openjdk.org/browse/JDK-8298276). But since this is not yet done, I propose to fix the test for now by just adding a short sleep during the recording to ensure the endChunk event is delayed enough to cause a new NMT snapshot to be taken.
**Testing** * Local testing with a larger `AgeThreshold` to see that a sleep really helps the problem. * Mach5 testing to verify the fix is good there as well, this is currently running.
Mach5 testing looks good, I plan to push this without waiting 24h to get rid of the CI noise. ------------- PR: https://git.openjdk.org/jdk/pull/11579
On Thu, 8 Dec 2022 07:58:39 GMT, Stefan Johansson <sjohanss@openjdk.org> wrote:
Please review this test fix.
**Summary** The new test TestNativeMemoryUsageEvents.java sometimes fail. The reason is that the _NativeMemoryUsage_ and _NativeMemoryUsageTotal_ events want to share a single `NMTUsage` snapshot. To achieve this there is an `AgeThreshold` set to 50ms that is considered when sending the events. If the usage snapshot is new enough we don't refresh it, but reuse it. In the test this becomes problematic if the recording is shorter than 50ms, then both the beginChunk event and the endChunk event will use the same NMT data.
This could be solved by using the feature currently reviewed in #11541 and this is the plan, see: [JDK-8298276](https://bugs.openjdk.org/browse/JDK-8298276). But since this is not yet done, I propose to fix the test for now by just adding a short sleep during the recording to ensure the endChunk event is delayed enough to cause a new NMT snapshot to be taken.
**Testing** * Local testing with a larger `AgeThreshold` to see that a sleep really helps the problem. * Mach5 testing to verify the fix is good there as well, this is currently running.
+1 ------------- Marked as reviewed by stuefe (Reviewer). PR: https://git.openjdk.org/jdk/pull/11579
On Thu, 8 Dec 2022 07:58:39 GMT, Stefan Johansson <sjohanss@openjdk.org> wrote:
Please review this test fix.
**Summary** The new test TestNativeMemoryUsageEvents.java sometimes fail. The reason is that the _NativeMemoryUsage_ and _NativeMemoryUsageTotal_ events want to share a single `NMTUsage` snapshot. To achieve this there is an `AgeThreshold` set to 50ms that is considered when sending the events. If the usage snapshot is new enough we don't refresh it, but reuse it. In the test this becomes problematic if the recording is shorter than 50ms, then both the beginChunk event and the endChunk event will use the same NMT data.
This could be solved by using the feature currently reviewed in #11541 and this is the plan, see: [JDK-8298276](https://bugs.openjdk.org/browse/JDK-8298276). But since this is not yet done, I propose to fix the test for now by just adding a short sleep during the recording to ensure the endChunk event is delayed enough to cause a new NMT snapshot to be taken.
**Testing** * Local testing with a larger `AgeThreshold` to see that a sleep really helps the problem. * Mach5 testing to verify the fix is good there as well, this is currently running.
This pull request has now been integrated. Changeset: 46cd457b Author: Stefan Johansson <sjohanss@openjdk.org> URL: https://git.openjdk.org/jdk/commit/46cd457b0f78996a3f26e44452de8f8a66041f58 Stats: 9 lines in 1 file changed: 9 ins; 0 del; 0 mod 8298341: Ensure heap growth in TestNativeMemoryUsageEvents.java Reviewed-by: egahlin, stuefe ------------- PR: https://git.openjdk.org/jdk/pull/11579
participants (3)
-
Erik Gahlin
-
Stefan Johansson
-
Thomas Stuefe