On Thu, 10 Nov 2022 19:54:30 GMT, Jorn Vernee <jver...@openjdk.org> wrote:

>> Jim Laskey has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Requested changes #5
>
> src/java.base/share/classes/java/lang/template/Carriers.java line 415:
> 
>> 413:          */
>> 414:         @Stable
>> 415:         private final Object[] objects;
> 
> Looking at all this, I'm skeptical that carriers are worth the complexity for 
> the performance gain they give. AFAICS the fast paths for `FMT` and `STR` 
> avoid boxing values into a StringTemplate instance altogether. So carriers 
> only seem to help cases where processing goes through the default `process` 
> method on a user-defined processor?
> 
> AFAIK `@Stable` will not do anything here. It only helps constant folding 
> from a constant root object and in general StringTemplate instances are not 
> constant. (and, AFAIK there are also JIT caveats with storing everything in 
> arrays like this).
> 
> I suggest maybe keeping things simple in this PR, and having just 
> `SimpleStringTemplate`. Public support for the linkage-based fast path can be 
> used to avoid the boxing for user defined processors in the future as well, 
> and that seems like a much better way to get performance to me.
> 
> If carriers are added to improve performance, I'd personally like to see that 
> in a separate PR that's only focused on that, with more benchmarks/bench 
> marking results included as well.

Fair comment. Initially, 99% of template processing will be through `STR``. 
However, that
will likely change as libraries expand to include template processing. In fact, 
creating
processors is so trivially easy that users will likely start using template 
processing for
every day tasks.

During the preview, string templates will be evaluatedby Java influencers for 
use in
future applications. If we come out of the gate with a less than stellar feature
then string templates will not gain traction.

Several reviewers, including most of the compiler team, had a problem with the 
use of
anonymous classes for each and every string template. Carriers were introduced 
as an
alternative. The fact that they perform well is an added bonus. Note Carriers 
are not a
last minute thing. I've been using them off/on for much of the project.

Carriers also have a place in future planned projects. It was Brian that 
inspired the idea.

Carriers, as well as the FormatProcessor, are indeed projects unto themselves, 
but are a
important part of the string template project. Breaking those projects out 
separately
would risk not making preview. It's important to get these classes tested in 
real use
cases during preview. In the balance, as you indicated, STR and FMT are the 
show. Carriers
are low risk with big win.

The linkage stuff will likely dissolve into something that doesn't require a 
lot of 
MethodHandle experience, something like a TemplateProcessorFactory.

I whipped up a trivial JMH benchmark to give a sense of Carrier worth. Note I'm 
not a
performance engineer and milage may vary, but this will give you the sense of 
degree
difference between Carriers and a list implementation.



public class MyBenchmark {
    // Declared globally to spoil optimization.
    static int x = 10, y = 20;
    static String result;
    static StringProcessor INTERPOLATE = st -> st.interpolate();
    static List<String> FRAGMENTS = List.of("", " + ", " = ", "");

    // String concatenation as a baseline.
    @Benchmark
    public void concat() {
        result = x + " + " + y + " = " + (x + y);
    }

    // STR is effectively String concatenation.
    @Benchmark
    public void str() {
        result = STR."{x} + {y} = {x + y}";
    }

    // StringTemplate using a carrier and thus the constructed interpolate.
    @Benchmark
    public void carrier() {
        result = INTERPOLATE."{x} + {y} = {x + y}";
    }

    // StringTemplates using lists for fragments and values. 
    // Note that FRAGMENTS is only constructed once as it would be in the wilds.
    @Benchmark
    public void lists() {
        StringTemplate st = StringTemplate.of(FRAGMENTS, List.of(x, y, x + y));
        result = st.interpolate();
    }

    // Formatting can be effectively String concatenation.
    @Benchmark
    public void format() {
        result = FMT."%d{x} + %d{y} = %d{x + y}";
    }

}




Benchmark                   Mode  Cnt         Score         Error  Units
MyBenchmark.concat         thrpt   25  52570142.770 ± 5373985.927  ops/s
MyBenchmark.str            thrpt   25  52569879.770 ± 2787317.140  ops/s
MyBenchmark.carrier        thrpt   25  54161402.682 ± 4587727.795  ops/s
MyBenchmark.lists          thrpt   25   9260123.476 ±  189741.261  ops/s
MyBenchmark.format         thrpt   25  50847393.625 ±  135673.614  ops/s


Thanks for pointing out the @Stable issue.

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

PR: https://git.openjdk.org/jdk/pull/10889

Reply via email to