RFR: 8344942: Template-Based Testing Framework [v76]

[Emanuel Peter]

On Tue, 3 Jun 2025 15:27:23 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> test/hotspot/jtreg/compiler/lib/template_framework/NameSet.java line 89:
>> 
>>> 87:             if (w < 0) {
>>> 88:                 throw new RuntimeException("Negative weight not allowed: " + w);
>>> 89:             }
>> 
>> I thought zero is also not allowed?
>
> Well that should have been filtered out already earlier, when we added the individual names. Now we could have a total weight of zero if we have no names. Then we just return `null`  here, and then throw an exception a little further up the use case chain, e.g. `DataName.sample` -> `throw new RendererException("No variable: " + mutability + msg1 + msg2 + ".");`
> 
> This here is just a sanity check, hence I throw a `RuntimeException`, and not a `RendererException`.

As asked for offline: added some more comments here.

>> test/hotspot/jtreg/compiler/lib/template_framework/Renderer.java line 358:
>> 
>>> 356:             // If the character was not found, we want to have the rest of the
>>> 357:             // String s, so instead of "-1" take the end/length of the String.
>>> 358:             dollar  = (dollar == -1)  ? s.length() : dollar;
>> 
>> `s.length()` could be called once before the loop and then reused inside the loop.
>
> You mean as a performance optimization? Is that not something we let the compiler do?

As discussed offline: I made `s` final, so we are sure that it is not mutated and the length should be moved out of the loop by the compiler. It would also only be a very small performance impact, as we are doing things like `indexOf` here, which are much more expensive anyway.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2124296212
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2124298706