RFR: JDK-8260282: Add option to compress heap dumps created by -XX:+HeapDumpOnOutOfMemoryError [v3]

[Thomas Stuefe]

On Tue, 26 Jan 2021 08:01:02 GMT, Ralf Schmelter <rschmelter at openjdk.org> wrote:

>> This change adds the optiont to created a gzipped heap dump by -XX:+HeapDumpOnOutOfMemoryError.
>> 
>> -XX:HeapDumpGzipLevel=<level> sets the compression level. 0 (the default) means no gzipping of the dump. Otherwise the level has to be between 1 and 10.
>
> Ralf Schmelter has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
> 
>  - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8260282
>  - Limit memory of heap dumping VM
>  - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8260282
>  - Fixed maximum supported compression level.
>  - Fixed trailing whitespace.
>  - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8260282
>  - Change allowed levels. 0 now means disabled.#
>  - Added option to compress the heap dump created by -XX:+HeapDumpOnOutOfMemoryError

Looks good. Nitpickings below, but I leave it up to you which ones you follow up to. Otherwise fine.

..Thomas

test/hotspot/jtreg/runtime/ErrorHandling/TestGZippedHeapDumpOnOutOfMemoryError.java line 28:

> 26:  * @summary Test verifies that -XX:HeapDumpGzipLevel=0 works
> 27:  * @library /test/lib
> 28:  * @run driver/timeout=240 TestGZippedHeapDumpOnOutOfMemoryError run 0

Is the timeout needed? Should the oome not be pretty much instantaneous?

test/hotspot/jtreg/runtime/ErrorHandling/TestGZippedHeapDumpOnOutOfMemoryError.java line 36:

> 34:  * @library /test/lib
> 35:  * @run driver/timeout=240 TestGZippedHeapDumpOnOutOfMemoryError run 1
> 36:  */

If the tests are fast enough, I would add at least another one at the end of the valid gzip range.

test/hotspot/jtreg/runtime/ErrorHandling/TestGZippedHeapDumpOnOutOfMemoryError.java line 74:

> 72:         OutputAnalyzer output = new OutputAnalyzer(proc);
> 73:         output.stdoutShouldNotBeEmpty();
> 74:         output.shouldContain("Dumping heap to " + heapdumpFilename);

Could this be a full path? In case printing on the VM side is ever changed to print the full path, I'd make this more lenient. Maybe two separate contains expressions or a `shouldMatch` with ".*" int he middle.

-------------

Marked as reviewed by stuefe (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2222