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

[Johan Sjölen]

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

>>> 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.

>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.

Thank you for explaining!

>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)

That's just differing taste then, thank you for taking the effort to try out my idea :-).

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

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