RFR: 8367531: Template Framework: use scopes and tokens instead of misbehaving immediate-return-queries
Emanuel Peter
epeter at openjdk.org
Mon Oct 13 07:21:28 UTC 2025
On Fri, 12 Sep 2025 10:16:10 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
> I got some feedback from users of the Template Framework, especially @galderz . And personally, I already was slightly unsatisfied by some of the issues described below, but did not expect it to be as bad as it is.
>
> So I'm sorry, but I think we need to do a significant re-design. It is now still early enough, and only trivial changes are required for the "real" uses of the framework. Only the framework internal tests require significant changes.
>
> Many thanks to @galderz for trying out the framework, and reporting the issues. And thanks to @chhagedorn for spending a few hours in an offline meeting discussing the issue.
>
> **Major issue with Template Framework: lambda vs token order**
>
> The template rendering involves some state, such as keeping track of hashtag replacements, names and fuel cost.
> Some methods have side-effects (`addDataName`, `let`, ...) and others are simple queries (`sample`, ...).
> Sadly, the first version of the template framework was not very consistent, and created tokens (deferred evaluation, during token evaluation) for some, and for others it queried the state and returned the result immediately (during lambda execution). One nasty consequence is that an immediately returning query can "float" above a state affecting token. For example, `addDataName` generated a token (so that we know if it is to be added for the template frame or a hook anchoring frame), but answered sampling queries immediately (because that means we can use the returned value immediately and make decisions based on it immediately, which is nice). Looking at the example below, this had the confusing result that `addDataName` only generates a token at first, then `sample` does not have that name available yet, and only later during token evaluation is the name actually added.
>
> var testTemplate = Template.make(() -> body(
> ...
> addDataName("name", someType, MUTABLE),
> let("name", dataNames(MUTABLE).exactOf(someType).sample().name()),
> ...
> ));
>
>
> **Two possible solutions: all-in on lambda execution or all-in on tokens**
>
> First, I thought I want to go all-in on lambda execution, and have everything have immediate effect and return results immediately. This would have the nice effect that the user feels like they are directly in control of the execution order. But I did not find a good way without exposing too many internals to the user, or getting rid of the nice "token lists" we currently have inside Templates (the list is directly concatenated). Look at the f...
Calling on the reviewers of the original Template Framework for review ;)
(of course there is no obligation, but you are most familiar with the code)
@mhaessig @robcasloz @chhagedorn @galderz
As mentioned above: I'm sorry that I did not get this right the first time. It is a lot of code, so feel free to ask for anything, including code-walk-throughs.
test/hotspot/jtreg/compiler/lib/template_framework/DataName.java line 126:
> 124: return fs.toString();
> 125: }
> 126: }
Having the `Predicate` interface allows unification of queries across `DataName` and `StructuralName`.
test/hotspot/jtreg/compiler/lib/template_framework/Hook.java line 70:
> 68: * public static int $field = 42;
> 69: * """
> 70: * )),
I decided to refactor the example using a scope insertion rather than template insertion. I think it is a bit more concise this way.
test/hotspot/jtreg/compiler/lib/template_framework/Renderer.java line 193:
> 191: return currentCodeFrame.listNames(predicate);
> 192: }
> 193:
We used to need these to access the queries from name filter set. But now we do it all from the Renderer anyway, so these methods are obsolete.
test/hotspot/jtreg/compiler/lib/template_framework/Renderer.java line 266:
> 264: case NothingToken() -> {
> 265: // Nothing.
> 266: }
Was needed for queries that returned immediately, but we wanted that users could place them in a token list, so everything looks nice. But now, all the queries are tokens that do something, so this is obsolete.
test/hotspot/jtreg/compiler/lib/template_framework/ScopeToken.java line 32:
> 30: * created with {@link Template#scope}, {@link Template#transparentScope}, and other related methods.
> 31: */
> 32: public sealed interface ScopeToken extends Token permits ScopeTokenImpl {}
Note: `ScopeToken` is a generalization of the old `TemplateBody`. Also: we need this to be public, because it needs to go into public interfaces like `Template.make`. But we don't want to expose the internals (e.g. we used to have tokens exposed, not great). So I decided to hide away the details in a `ScopeTokenImpl` class now.
test/hotspot/jtreg/compiler/lib/template_framework/SetFuelCostToken.java line 29:
> 27: * Represents the setting of the fuel cost in the current scope.
> 28: */
> 29: record SetFuelCostToken(float fuelCost) implements Token {}
Github presents this as a renaming, but it is really a removal of `NothingToken` (obsolete), and an addition of the new `SetFuelCostToken`.
test/hotspot/jtreg/compiler/lib/template_framework/Template.java line 182:
> 180: * {@link Hook#insert}ed to where a {@link Hook} was {@link Hook#anchor}ed earlier (in some outer scope of the code).
> 181: * For example, while generating code in a method, one can reach out to the scope of the class, and insert a
> 182: * new field, or define a utility method.
Note: removed, replaced by new descriptions about scopes and hook usage.
test/hotspot/jtreg/compiler/lib/template_framework/Template.java line 195:
> 193: * template can be changed when we {@code render()} it (e.g. {@link ZeroArgs#render(float)}) and the default
> 194: * fuel cost with {@link #setFuelCost}) when defining the {@link #body(Object...)}. Recursive templates are
> 195: * supposed to terminate once the {@link #fuel} is depleted (i.e. reaches zero).
Note: moved down
test/hotspot/jtreg/compiler/lib/template_framework/Template.java line 225:
> 223: * of the lambda. But a method like {@link #addDataName} returns a token, and does not immediately add
> 224: * the {@link DataName}. This ensures that the {@link DataName} is only inserted when the tokens are
> 225: * evaluated, so that it is inserted at the exact scope where we would expect it.
Note: this was really the confusing part about the Template Framework, and this PR addresses the problem by making everything tokens.
test/hotspot/jtreg/compiler/lib/template_framework/Template.java line 267:
> 265: * // one by one.
> 266: * ));
> 267: * }
Example is now obsolete, and not correct any more. It used to demonstrate the confusing behavior of immediate-return queries mixed with delayed `addDataName`, which could mean that `count` would not see `addDataName` that happened above it in the same template. That is now fixed with this PR.
test/hotspot/jtreg/compiler/lib/template_framework/Template.java line 846:
> 844: */
> 845: static <T> Token let(String key, T value, Function<T, ScopeToken> function) {
> 846: return new LetToken(key, value, function);
Note: this special version of `let` was constrained to the top of a Template, and would only forward the `body`. It was a bit strange, and I think now it is nicer: you can use it at any point in the template and do a computation, and have the hashtag and lambda argument available for the inner scope.
test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestTutorial.java line 79:
> 77: comp.addJavaSourceCode("p.xyz.InnerTest7", generateWithDataNamesSimple());
> 78: comp.addJavaSourceCode("p.xyz.InnerTest8", generateWithDataNamesForFieldsAndVariables());
> 79: comp.addJavaSourceCode("p.xyz.InnerTest9", generateWithScopes1());
These used to demonstrate the old "bad" behaviour: queries could float above `addDataName`. Now I replaced it with new tutorial "chapters".
test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestTutorial.java line 423:
> 421: int #name = #value;
> 422: """
> 423: ));
I think it is preferable to use scope insertion for the tutorial. It makes the example a little more readable.
test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestTutorial.java line 594:
> 592: #f += 42;
> 593: """
> 594: ));
Note: I inlined the code directly. Having a separate template was not great.
test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestTutorial.java line 699:
> 697: let("longs", dataNames(MUTABLE).exactOf(myLong).count()),
> 698: // Note: we could also count the MUTABLE_OR_IMMUTABLE, we will
> 699: // cover the concept of mutability in an example further down.
Note: query used to return the integer count immediately. That meant that `count` could float above a `addDataName`. Now we do it with a token instead, and so it looks a little less convenient, since we have to capture the value into a lambda argument. See new code.
test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestTutorial.java line 812:
> 810: // Let us have a closer look at how DataNames interact with scopes created by
> 811: // Templates and Hooks. Additionally, we see how the execution order of the
> 812: // lambdas and token evaluation affects the availability of DataNames.
Note: `generateWithDataNamesAndScopes1/2` demonstrate the bad "old" way. Now I replaced it with `generateWithScopes1`, that explains the scopes and queries that relate to scopes.
test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestTutorial.java line 833:
> 831: // so that the DataName can escape to outer scopes.
> 832: var templateStaticField = Template.make("type", (DataName.Type type) -> transparentScope(
> 833: addDataName($("field"), type, MUTABLE), // escapes template because of "transparentScope"
Note: `Hook.insert` used to have an implicitly transparent scope ... but I did not like that. I think it is preferable to specify transparency explicitly, and so that is what we do now.
test/hotspot/jtreg/testlibrary_tests/template_framework/tests/TestTemplate.java line 483:
> 481: hook1.insert(template1.asToken()),
> 482: hook1.insert(scope("Beautiful\n", template0.asToken())),
> 483: "t2 isAnchored: ", hook1.isAnchored(a -> scope(a)), "\n"
Note: the `isAnchored` query here would have floated above the `anchor` before this PR, and given us misleading results!
-------------
PR Comment: https://git.openjdk.org/jdk/pull/27255#issuecomment-3396166822
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425289812
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425298309
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425303196
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425307159
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425313590
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425315237
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425319022
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425318168
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425320302
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425323288
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425327712
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425333598
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425341015
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425342161
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425350222
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425352394
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425345031
PR Review Comment: https://git.openjdk.org/jdk/pull/27255#discussion_r2425359071
More information about the hotspot-compiler-dev
mailing list