RFR: 8316328: Shenandoah: jdk/jfr/event/oldobject/TestSanityDefault.java times out for some heap sizes
This is a request to limit the memory used by the test, otherwise it could run "forever" on huge heaps. ------------- Commit messages: - Update TestSanityDefault.java - 8316328: Shenandoah: jdk/jfr/event/oldobject/TestSanityDefault.java times out for some heap sizes Changes: https://git.openjdk.org/jdk/pull/19163/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19163&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8316328 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19163.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19163/head:pull/19163 PR: https://git.openjdk.org/jdk/pull/19163
On Fri, 10 May 2024 00:08:39 GMT, Sergey Bylokhov <serb@openjdk.org> wrote:
This is a request to limit the memory used by the test, otherwise it could run "forever" on huge heaps.
Marked as reviewed by phh (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/19163#pullrequestreview-2069434112
On Fri, 10 May 2024 00:08:39 GMT, Sergey Bylokhov <serb@openjdk.org> wrote:
This is a request to limit the memory used by the test, otherwise it could run "forever" on huge heaps.
test/jdk/jdk/jfr/event/oldobject/TestSanityDefault.java line 40:
38: * @library /test/lib /test/jdk 39: * @summary Purpose of this test is to run leak profiler without command line tweaks or WhiteBox hacks until we succeed 40: * @run main/othervm -mx64m jdk.jfr.event.oldobject.TestSanityDefault
Should it be `-Xmx`? I understand `-mx` is de jure standard, but we use `-Xmx` everywhere. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19163#discussion_r1609580957
On Wed, 22 May 2024 09:03:06 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
This is a request to limit the memory used by the test, otherwise it could run "forever" on huge heaps.
test/jdk/jdk/jfr/event/oldobject/TestSanityDefault.java line 40:
38: * @library /test/lib /test/jdk 39: * @summary Purpose of this test is to run leak profiler without command line tweaks or WhiteBox hacks until we succeed 40: * @run main/othervm -mx64m jdk.jfr.event.oldobject.TestSanityDefault
Should it be `-Xmx`? I understand `-mx` is de jure standard, but we use `-Xmx` everywhere.
I am thinking if 64M is a bit tight. For this test, we add `OldObjects.MIN_SIZE` (~100K) reachable objects in the list on every iteration. Ballparking each object having a marginal footprint of ~20 bytes, this means we allocate about 2M of data on every iteration. Suppose the previous iteration did not promote the object to old yet, but the very next allocation would. We would still need to absorb ~2M allocation into old, lest we run into OOM. Which means this thread implicitly relies on old being at least 2M, which might be not given for 64M heap. In practice, GCs would probably trash sooner. So, maybe capping at more reasonable 1G would suffice to get reasonable test times without going into very tight heaps? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19163#discussion_r1609612990
On Wed, 22 May 2024 09:24:02 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
test/jdk/jdk/jfr/event/oldobject/TestSanityDefault.java line 40:
38: * @library /test/lib /test/jdk 39: * @summary Purpose of this test is to run leak profiler without command line tweaks or WhiteBox hacks until we succeed 40: * @run main/othervm -mx64m jdk.jfr.event.oldobject.TestSanityDefault
Should it be `-Xmx`? I understand `-mx` is de jure standard, but we use `-Xmx` everywhere.
I am thinking if 64M is a bit tight. For this test, we add `OldObjects.MIN_SIZE` (~100K) reachable objects in the list on every iteration. Ballparking each object having a marginal footprint of ~20 bytes, this means we allocate about 2M of data on every iteration. Suppose the previous iteration did not promote the object to old yet, but the very next allocation would. We would still need to absorb ~2M allocation into old, lest we run into OOM. Which means this thread implicitly relies on old being at least 2M, which might be not given for 64M heap. In practice, GCs would probably trash sooner.
So, maybe capping at more reasonable 1G would suffice to get reasonable test times without going into very tight heaps?
The patch is updated as suggested ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19163#discussion_r1610903205
This is a request to limit the memory used by the test, otherwise it could run "forever" on huge heaps.
Sergey Bylokhov has updated the pull request incrementally with one additional commit since the last revision: Update TestSanityDefault.java ------------- Changes: - all: https://git.openjdk.org/jdk/pull/19163/files - new: https://git.openjdk.org/jdk/pull/19163/files/aff79f02..3a0cb032 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19163&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19163&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19163.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19163/head:pull/19163 PR: https://git.openjdk.org/jdk/pull/19163
On Thu, 23 May 2024 02:30:13 GMT, Sergey Bylokhov <serb@openjdk.org> wrote:
This is a request to limit the memory used by the test, otherwise it could run "forever" on huge heaps.
Sergey Bylokhov has updated the pull request incrementally with one additional commit since the last revision:
Update TestSanityDefault.java
Looks good to me. @egahlin, would you like to ack as well? ------------- Marked as reviewed by shade (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19163#pullrequestreview-2073316106
On Thu, 23 May 2024 02:30:13 GMT, Sergey Bylokhov <serb@openjdk.org> wrote:
This is a request to limit the memory used by the test, otherwise it could run "forever" on huge heaps.
Sergey Bylokhov has updated the pull request incrementally with one additional commit since the last revision:
Update TestSanityDefault.java
Marked as reviewed by egahlin (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/19163#pullrequestreview-2073329421
On Thu, 23 May 2024 02:30:13 GMT, Sergey Bylokhov <serb@openjdk.org> wrote:
This is a request to limit the memory used by the test, otherwise it could run "forever" on huge heaps.
Sergey Bylokhov has updated the pull request incrementally with one additional commit since the last revision:
Update TestSanityDefault.java
One minor nit: This bug does not seem to be caused by Shenandoah, it is just well triggered by it. @sendaoYan had a sighting with Parallel, see the comment in the bug. Maybe we should drop `Shenandoah: ` from synopsis before we integrate the fix. ------------- PR Comment: https://git.openjdk.org/jdk/pull/19163#issuecomment-2126665545
On Fri, 10 May 2024 00:08:39 GMT, Sergey Bylokhov <serb@openjdk.org> wrote:
This is a request to limit the memory used by the test, otherwise it could run "forever" on huge heaps.
This pull request has now been integrated. Changeset: 9b1d6d66 Author: Sergey Bylokhov <serb@openjdk.org> URL: https://git.openjdk.org/jdk/commit/9b1d6d66b8297d53c6b96b9e2f9bd69af90ab8fb Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8316328: Test jdk/jfr/event/oldobject/TestSanityDefault.java times out for some heap sizes Reviewed-by: phh, shade, egahlin ------------- PR: https://git.openjdk.org/jdk/pull/19163
participants (4)
-
Aleksey Shipilev
-
Erik Gahlin
-
Paul Hohensee
-
Sergey Bylokhov