[ 
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14005756#comment-14005756
 ] 

Jochen Kemnade commented on TAP5-2332:
--------------------------------------

I think we should try to make all parts of the code fast, regardless of whether 
they are called often or just once in a while, but I don't insist on optimizing 
OptionModelImpl.
I'd really like to get this closed soon, because I think we've already spent 
way too much time discussing about how we want to concatenate strings.
I guess, we have proved well enough that the StringBuilder approach is faster 
that using String.format, so I'll assume that everyone agrees in that regard.
I propose the following:
I'll create a new patch tonight, including my proposed concat method. I will 
add it to {{InternalUtils}} even though I think that  {{IOCUtilities}} would be 
the right spot (the Javadoc describes the class as "A collection of utility 
methods for a couple of different areas").
I'll replace the places that Michael's patch addresses (not including 
OptionModelImpl even though I think that it should also be changed).
I'll attach the patch along with some new measurements so we're all discussing 
the exact same thing.

> Optimize String concatenation performance
> -----------------------------------------
>
>                 Key: TAP5-2332
>                 URL: https://issues.apache.org/jira/browse/TAP5-2332
>             Project: Tapestry 5
>          Issue Type: Improvement
>            Reporter: Michael Mikhulya
>            Assignee: Jochen Kemnade
>            Priority: Minor
>              Labels: performance
>         Attachments: 0001-TAP5-2332-get-rid-of-String.format-usage.patch, 
> profile-patched.png, profile-vanilla.png
>
>
> During profiling I found that String.format provides much load on CPU.
> In many cases in Tapestry String.format can be easily replaced with simple 
> String concatenation.
> Simple JMH (http://openjdk.java.net/projects/code-tools/jmh/) test
> {code:java}
> public class FormatVsConcat {
>     private static final String format = "This is a test string with %s";
>     private static final String concat1 = "This is a test string with ";
>     private static final String concat2 = "test word";
>     @GenerateMicroBenchmark
>     public String format() {
>         return String.format(format, concat2);
>     }
>     @GenerateMicroBenchmark
>     public String concat() {
>         return concat1 + concat2;
>     }
> }
> {code}
> shows, that concatenation is 366(!) times faster.
> I removed only hot places in tapestry and get following results with apache 
> benchmark:
> *Not patched* tapestry version:
> Requests per second: *21.38 /sec* (mean)
> Time per request: *46.764 [ms]* (mean)
> *Patched* tapestry version:
> Requests per second: *27.77 /sec* (mean)
> Time per request: *36.013 [ms]* (mean)
> So we gained 10ms per request or 20% of rendering time.
> If you don't mind I would like to get rid of String.format in all places of 
> Tapestry and provide patch. I fixed only hot places which appeared during 
> ab-profiling of one concrete page. So it is very likely that not all hot 
> places were found and fixed.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to