RFR: 8331185: Enable compiler memory limits in debug builds [v8]

[Ashutosh Mehra]

On Thu, 2 May 2024 13:54:08 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> See [1] for previous discussions.
>> 
>> We'd like to introduce a default memory limit for compilations in debug builds. That way, we can catch pathological compiler errors that have an unreasonably high per-compilation memory footprint early during testing.
>> 
>> The default limit affects all compilations, unless the method is subject to a memory limit set from command line. Meaning, `-XX:CompileCommand=MemLimit,...` overrules the default.
>> 
>> Examples:
>> 
>> This lowers the memlimit for j.l.String methods - all methods will have the default 1GB limit in a debug JVM. Only j.l.String will run with a 100M limit: `-XX:CompileCommand=MemLimit,java.lang.String::*,100m`
>> 
>> This disables the default memlimit globally: `-XX:CompileCommand=MemLimit,*.*,0`
>> 
>> 
>> ---
>> 
>> The patch:
>> 
>> 1) adds a debug-only default memory limit of **1GB** (as proposed by @vnkozlov). The limit action is "crash", meaning we will assert.
>> 2) To test the mechanics, we now print out the memory limit for each compilation in the compilation cost record.
>> 3) Adapted and extended tests
>> 
>> I also fixed up some copyrights that I overlooked last year when adding the compiler memory statistics this patch builds atop of.
>> 
>> 
>> Tested:
>> 
>> - manually on Mac m1 (debug and release)
>> - GHAs are running
>> - but Oracle will do more testing before this goes in
>> 
>> [1] https://mail.openjdk.org/pipermail/hotspot-compiler-dev/2024-April/074787.html
>
> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove unused variable

Marked as reviewed by asmehra (Committer).

Just one suggestion which you may pick or ignore. Otherwise looks good.

test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java line 143:

> 141:             // total     NA        RA        result  #nodes  limit   time    type  #rc thread              method
> 142:             // 32728     0         32728     ok     -       1024M   0.045   c1    1   0x000000011b019c10  compiler/print/CompileCommandMemLimit$TestMain::method1(()J)
> 143:             oa.shouldMatch("\\d+ +\\d+ +\\d+ +ok +" + numberNodesRegex + " +" + implicitMemoryLimit + " +.* +" + method1regex);

A minor suggestion regarding the regex. I find "\s+" more readable than " +" to match multiple spaces.

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

PR Review: https://git.openjdk.org/jdk/pull/18969#pullrequestreview-2036760008
PR Comment: https://git.openjdk.org/jdk/pull/18969#issuecomment-2091437900
PR Review Comment: https://git.openjdk.org/jdk/pull/18969#discussion_r1588272234