[jira] [Comment Edited] (TAP5-2332) Get rid of String.format usage

["Thiago H. de Paula Figueiredo (JIRA)" <ji...@apache.org>]

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

Thiago H. de Paula Figueiredo edited comment on TAP5-2332 at 5/19/14 2:30 PM:
------------------------------------------------------------------------------

First of all, I think you should profile a Tapestry application first to know how much page rendering time is actually spent on String.format() calls *inside the framework, not inside pages*, and which ones exactly. It's not clear in the ticket description whether this was done or not. Otherwise, it'll probably fall into premature optimization. I do suspect it the time spent of String.format() will be very low; In addition, removing all String.format() calls is completely out of question. It would be trading trading flexibility for a small performance gain. An awful lot of messages can be customized through property file entires using String.format(). On the other hand, the patch shows some places in which String.format() could be replaced without loss of functionality.

Anyway, thanks for investigating this.


was (Author: thiagohp):
First of all, I think you should profile a Tapestry application first to know how much page rendering time is actually spent on String.format() calls inside the framework, not inside pages, and which ones. It's not clear in the ticket description whether this was done or not. Otherwise, it'll probably fall into premature optimization. I do suspect it the String.format() will be very low; In addition, removing all String.format() calls is completely out of question. It would be trading trading flexibility for a small performance gain. An awful lot of messages can be customized through property file entires using String.format().

Anyway, thanks for investigating this.

> Get rid of String.format usage
> ------------------------------
>
>                 Key: TAP5-2332
>                 URL: https://issues.apache.org/jira/browse/TAP5-2332
>             Project: Tapestry 5
>          Issue Type: Improvement
>            Reporter: Michael Mikhulya
>            Priority: Minor
>              Labels: performance
>         Attachments: 0001-TAP5-2332-get-rid-of-String.format-usage.patch
>
>
> 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)