[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14006045#comment-14006045
]
Michael Mikhulya commented on TAP5-2332:
----------------------------------------
IMO if I know that some job can be done in different ways: A, B, C.
And I measured that way A is the fastest way, than I'm going *always* use that
way.
In many cases it is hard to say whether code will be hot or not. And the same
code can be hot or not dependently on concrete benchmark.
I don't see why I should use old way (B or C) when it is known to be slower
than A and it doesn't provide *any* benefits.
Regarding Knuth: there is no difference in debugging and maintenance between
these two cases:
{code:java}
String.format("some text %s %s", param1, param2);
InternalUtils.concat("some text ", param1, " ", param2);
{code:java}
InternalUtils.concat has several benefits:
1. It is much faster
2. When it is seen in code then it provides knowledge to other developers. So
it decrease risks to have the same problem in already fixed code and decrease
risks of appearence the same problem in new code.
3. It allows to improve things by changing only one place. (It is easy to
switch implementation).
Can anybody explain why this easy simple change can't be applied in all places?
If you don't have much time to create a patch then I will create it. It will
not take much time and there is no risk to make any regressions.
> 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)
