RFR: JDK-8320061: [nmt] Multiple issues with peak accounting [v2]

[Thomas Stuefe]

On Fri, 24 Nov 2023 16:12:50 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> test/lib/jdk/test/lib/process/OutputAnalyzer.java line 822:
>> 
>>> 820:             }
>>> 821:             throw new RuntimeException(err);
>>> 822:         }
>> 
>> Splitting this up into multiple loops would simplify the code significantly imho, no need for two boolean flags for a start. Also, what does the `!verbose` check do exactly? Doesn't seem like it would be print twice without it, to me.
>> 
>> 
>> if (needles.length == 0) return;
>> 
>> int haystackStart = 0;
>> for (int i = 0; i < haystack.length; i++) {
>>      if (haystack[i].contains(needles[0])) {
>>       hayStackStart = i+1;
>>       return;
>>     }
>> }
>> 
>> for (int i = 1; i < needles.length; i++) {
>>     if (verbose && haystackStart + i < haystack.length) {
>>         System.out.println("" + haystackStart + i + ":" + haystack[haystackStart + i]);
>>     }
>>     if (haystackStart + i >= haystack.length
>>         || !haystack[haystackStart + i].contains(needles[i])) {
>>         String err = "First unmatched: "" + needles[needleIdx] + """;
>>         if (!verbose) { // don't print twice
>>             reportDiagnosticSummary();
>>         }
>>         throw new RuntimeException(err);
>>     }
>>     if (verbose) {
>>         System.out.println("Matches pattern " + needleIdx + " ("" + haystack[haystackStart + i] + "")");
>>     }
>> }
>
>> Splitting this up into multiple loops would simplify the code significantly imho, no need for two boolean flags for a start. 
> 
> If verbose=true, we print the haystack, with pattern matches marked. Your version omits printing for all lines until a needle is found, and only prints from the second needle on. Meaning, if the first pattern does not match, we don't see output. Arguably, that is the point where you need the output for test analysis.
> 
> Okay, I can give it a try.
> 
>> Also, what does the !verbose check do exactly? Doesn't seem like it would be print twice without it, to me.
> 
> If an error happened, we print diagnostic summary (which mimics the behavior of all other "shouldBlaBla()" functions). So, we print twice if we run with verbose and run into an error.

Okay, I rewrote it in the style you prefer, while keeping the logs I wanted intact; this does not look much clearer to me tbh. I found the first version more elegant, since it was a one-loop state machine and could be easily adapted to other uses (e.g. scanning for pattern that are not in directly consecutive lines, but interspersed)

No matter. I got it to work, lets keep it this way.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16675#discussion_r1404530803