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

[Emanuel Peter]

On Tue, 3 Jun 2025 14:05:14 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   rename View -> FilteredSet
>
> 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`.

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

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2124241074
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2124247590