[jira] [Comment Edited] (TAP5-2332) Optimize String concatenation performance

["Michael Mikhulya (JIRA)" <ji...@apache.org>]

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

Michael Mikhulya edited comment on TAP5-2332 at 8/2/14 5:16 AM:
----------------------------------------------------------------

Seems like my original idea can be improved.
It would be better to make string concatenation (or formatting) only when needed. 

Such idea is integrated into java8 in many places.
For example look at
http://docs.oracle.com/javase/8/docs/api/java/util/logging/Logger.html#severe-java.util.function.Supplier-

So now you can log as follows:
{code:java}
logger.severe(() -> String.format(...))
{code}

I suggest to use the same approach here, but use Callable<String> instead of Supplier<String> at least until we don't migrate to Java8.

I will be on vacation the next 3 weeks. After it I can provide new patch and benchmark results if all agree with new aproach. 


was (Author: mihasik):
Seems like my original idea can be improved.
It would be better to make string concatenation (or formatting) only when needed. 

Such idea is integrated into java8 in many places.
For example look at
http://docs.oracle.com/javase/8/docs/api/java/util/logging/Logger.html#severe-java.util.function.Supplier-

So now you can log as follows:
{code:java}
logger.severe(() -> String.format(...))
{code}

I suggest to use the same approach here, but use Callable<String> instead of Supplier<String> at least until we don't migrate to Java8.

I will be on vacation the next 3 weeks. After it I can provide new patch and benchmark results if all agree whith new aproach. 

> 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
>            Priority: Minor
>              Labels: patch, performance
>         Attachments: 0001-TAP5-2332-Add-a-method-for-optimized-String-concaten.patch, 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)