[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14237126#comment-14237126
]
ASF subversion and git services commented on TAP5-2332:
---
Commit d871f66b38188bdc6a94290729a93279465fd81b in tapestry-5's branch
refs/heads/master from [~jkemnade]
[ https://git-wip-us.apache.org/repos/asf?p=tapestry-5.git;h=d871f66 ]
TAP5-2332: Optimize
org.apache.tapestry5.internal.transform.EventHandlerMethodParameterSource.get(ComponentEvent,
int) (~+30% reqs/second)
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: patch, performance
Fix For: 5.4
Attachments:
0001-TAP5-2332-Add-a-method-for-optimized-String-concaten.patch,
0001-TAP5-2332-Replace-String.format-call-by-simple-Strin.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.3.4#6332)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14237137#comment-14237137
]
Hudson commented on TAP5-2332:
--
SUCCESS: Integrated in tapestry-trunk-freestyle #1354 (See
[https://builds.apache.org/job/tapestry-trunk-freestyle/1354/])
TAP5-2332: Optimize
org.apache.tapestry5.internal.transform.EventHandlerMethodParameterSource.get(ComponentEvent,
int) (~+30% reqs/second) (jkemnade: rev
d871f66b38188bdc6a94290729a93279465fd81b)
*
tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/EventHandlerMethodParameterSource.java
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: patch, performance
Fix For: 5.4
Attachments:
0001-TAP5-2332-Add-a-method-for-optimized-String-concaten.patch,
0001-TAP5-2332-Replace-String.format-call-by-simple-Strin.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.3.4#6332)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14209476#comment-14209476
]
ASF subversion and git services commented on TAP5-2332:
---
Commit 4de73e9642263d8f5b02000663a196df46059b99 in tapestry-5's branch
refs/heads/master from [~mihasik]
[ https://git-wip-us.apache.org/repos/asf?p=tapestry-5.git;h=4de73e9 ]
TAP5-2332: Replace String.format call by simple String concatenation, the
compiler will transform that to StringBuilder-based concatenation which is
considerably faster
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-Replace-String.format-call-by-simple-Strin.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.3.4#6332)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14209499#comment-14209499
]
Hudson commented on TAP5-2332:
--
SUCCESS: Integrated in tapestry-trunk-freestyle #1346 (See
[https://builds.apache.org/job/tapestry-trunk-freestyle/1346/])
TAP5-2332: Replace String.format call by simple String concatenation, the
compiler will transform that to StringBuilder-based concatenation which is
considerably faster (jochen.kemnade: rev
4de73e9642263d8f5b02000663a196df46059b99)
*
tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinkerImpl.java
*
tapestry-core/src/main/java/org/apache/tapestry5/internal/beaneditor/MessagesConstraintGenerator.java
*
tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/ComponentPageElementImpl.java
* tapestry-core/src/main/java/org/apache/tapestry5/internal/OptionModelImpl.java
*
tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PerThreadServiceLifecycle.java
*
tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/JavaScriptStackPathConstructorImpl.java
*
tapestry-core/src/main/java/org/apache/tapestry5/internal/services/EventImpl.java
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: patch, performance
Fix For: 5.4
Attachments:
0001-TAP5-2332-Add-a-method-for-optimized-String-concaten.patch,
0001-TAP5-2332-Replace-String.format-call-by-simple-Strin.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.3.4#6332)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14137001#comment-14137001
]
Michael Mikhulya commented on TAP5-2332:
[~jkemnade] your latest patch is absolutely the same as my original patch, but
doesn't contain changes for RenderQueueImpl.java and RenderQueueImplTest.java.
Can we apply these changes too?
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-Replace-String.format-call-by-simple-Strin.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.3.4#6332)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14137077#comment-14137077
]
Jochen Kemnade commented on TAP5-2332:
--
Those changes have already been applied in
b452c6fcdc68c3afcf89faf5d21bd5508ea7bd25.
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-Replace-String.format-call-by-simple-Strin.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.3.4#6332)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14135493#comment-14135493
]
Jochen Kemnade commented on TAP5-2332:
--
[~thiagohp], [~mihasik], what do you think? Is the latest patch something we
can agree upon?
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-Replace-String.format-call-by-simple-Strin.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.3.4#6332)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14128200#comment-14128200
]
Lance commented on TAP5-2332:
-
As before, I'd be highly surprised if {{OptionModelImpl}} really makes a
difference to performance unless you have thousands of select options.
I'm fine with this, but I really hope that just because you've found that
{{String.format(...)}} is slower than concatenation you don't stop using it. We
all know that `String.format(...)` is slower than StringBuilder, it has to be
since it's doing more.
{{String.format(...)}} is convenient and readable and I for one won't stop
using because it's a teensy bit slower. I'll only optimise once a hotspot has
been found.
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-Replace-String.format-call-by-simple-Strin.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.3.4#6332)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14128214#comment-14128214
]
Lance commented on TAP5-2332:
-
{quote}Concatenation with {{+}} is more readable{quote}
I couldn't disagree more
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-Replace-String.format-call-by-simple-Strin.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.3.4#6332)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14128235#comment-14128235
]
Jochen Kemnade commented on TAP5-2332:
--
I don't intend to do that...
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-Replace-String.format-call-by-simple-Strin.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.3.4#6332)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14128510#comment-14128510
]
Michael Mikhulya commented on TAP5-2332:
Jochen (or somebody else), do you have time to measure performance in case of
lazy calculation of description inside OperationTracker methods?
Actually OperationTracker is the hotest place and it would be great to avoid
concatenation at all when possible.
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-Replace-String.format-call-by-simple-Strin.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.3.4#6332)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14128532#comment-14128532
]
Jochen Kemnade commented on TAP5-2332:
--
I won't do it. I'm tired of performing measurements. I just want to find a
solution that everyone can live with and close this issue for good.
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-Replace-String.format-call-by-simple-Strin.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.3.4#6332)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14113652#comment-14113652
]
Jochen Kemnade commented on TAP5-2332:
--
{quote}
Seems like my original idea can be improved.
It would be better to make string concatenation (or formatting) only when
needed.
{quote}
If we were really going to make it lazy, I'd still favor building it on top of
CharSequence, something like Rhino's ConsString.
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14105235#comment-14105235
]
Jochen Kemnade commented on TAP5-2332:
--
{{org.apache.tapestry5.internal.services.EventImpl.storeResult(Object)}} seems
to be quite slow too.
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14083376#comment-14083376
]
Michael Mikhulya commented on TAP5-2332:
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 CallableString instead of
SupplierString 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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14005713#comment-14005713
]
Jochen Kemnade commented on TAP5-2332:
--
Alright. I did some profiling. 1000 requests to tapestry-core:test:app1/
To demonstrate the effect, I profiled only the
{{org.apache.tapestry5.internal.services.EventImpl}} class and the only thing I
changed between the runs is the {{description}} argument to the
{{tracker.invoke}} call. I changed it from {{String.format}} to simple string
concatenation with {{+}}. The time taken by the {{storeResult}} method went
from 950ms down to ~66ms.
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
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14005726#comment-14005726
]
Lance commented on TAP5-2332:
-
Great, so is your patch just going to change `EventImpl`?
I notice that this class has been performance tuned before
{code}
// TAP5-471: Thousands of calls to isDebugEnabled() do add up
debugEnabled = logger.isDebugEnabled();
{code}
I'm a lot happier to tune this hot-spot rather than all `String.format(...)`
calls.
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14005728#comment-14005728
]
Jochen Kemnade commented on TAP5-2332:
--
Nobody wants to replace all String.format calls, only the ones that simply
concatenate strings. We're going in circles here.
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14005738#comment-14005738
]
Lance commented on TAP5-2332:
-
I only want to change the things that actually make a difference. I feek that
EventImpl is a hot-spot so I can definately see that making a difference. I
would be very surprised if, for instance, OptionModelImpl was a hot-spot.
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14005868#comment-14005868
]
Thiago H. de Paula Figueiredo commented on TAP5-2332:
-
Jochen, you said: 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. I'm
sorry, but I need to be honest: what you said is wrong. Very wrong. Optimizing
code that is invoked just once in a while will have absolutely no improvement
in performance and all changes have a risk of introducing bugs in a mature
codebase. Correctness is more important then performance. It's a very known
rule about optimization that usually 80% of the running time is spent on just
20% of the code (of course, numbers vary). So, if you cut in half the running
time of the other 80%, of the code, you only get a 10% improvement. It's too
much work for little result. I think that's what Lance is saying. We should
only change something for performance if it'll actually make a difference.
IoCUtilities is the wrong place to put the concat() method because, as it's
name says, it's utilities for IoC. Cohesion! It would be very weird, not to say
wrong, to have concat() besides buildDefaultRegistry() and addDefaultModules().
Jochen, please don't commit code until we agree what needs to be optimized. We
have, so far, discussed a lot what needs to be changed, not how to concatenate
strings.
Jochen and Lance: send an e-mail to sa...@yourkit.com mentioning your
@apache.org e-mail address and they'll send you an YourKit Java profiler
license you can use to work on Tapestry. They answer quickly: I've asked
yesterday evening, got it today morning. By the way, thanks YourKit!
Again, request times are less important then knowing the hotspots so we only
change what will make a difference and leave code that won't make a difference
unchanged.
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14005892#comment-14005892
]
Jochen Kemnade commented on TAP5-2332:
--
I tried to express and explain my opinions on the current situation, regarding
both the optimizations and the impact of the proposed solutions, and my
proposal to add the concat method to IOCUtilities. I don't share everyone
else's opinions on the topic, but I wouldn't call their opinions wrong anyway.
I didn't say I wanted to commit anything, far from it! Again, I'll create a
patch and new measurements tonight. I can do some basic profiling but I'll do
it with visualvm. I'll attach the results to the issue.
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14005923#comment-14005923
]
Thiago H. de Paula Figueiredo commented on TAP5-2332:
-
Jochen, in this case, I said your opinion is wrong because it goes against
almost everything which was said about code optimization in the last decades.
http://c2.com/cgi/wiki?PrematureOptimization is a good starting point. See what
Donald Knuth, one of the most important people in software development
research, has to say about this: Programmers waste enormous amounts of time
thinking about, or worrying about, the speed of noncritical parts of their
programs, and these attempts at efficiency actually have a strong negative
impact when debugging and maintenance are considered. We should forget about
small efficiencies, say about 97% of the time: premature optimization is the
root of all evil. Yet we should not pass up our opportunities in that critical
3%.
By the way, the patch includes a change in a test. That's code that isn't used
in Tapestry in runtime at all. It's not even included in the JAR. So this was a
completely wasted effort in optimization. AbstractOptionModel.toString() is
another one. The change in MessageConstraintsGenerator may actually be a
serious bug, but I couldn't check yet. We need to optimize hotspots, the 3%
mentioned by Knuth, and leave the rest alone.
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14005943#comment-14005943
]
Jochen Kemnade commented on TAP5-2332:
--
Yes, and if the test hadn't been adapted to the new behavior of the main source
set, it would have failed. The change in the test file doesn't even have
something to do with string concatenation and it is unrelated to the discussion.
And I think that Knuth's quote can only applied here with some force. It's not
as if I wanted to introduce 100 lines of cryptic code to solve some tiny part
of a complicated problem. It's about /string concatenation/. The code I
proposed is simple, really simple. And it can be tested very well. Besides, if
you put it like that, String.format is also premature optimization of
concatenating with {{+}}.
And please don't comment on that old patch anymore, I said I'll create a new
one (without OptionModelImpl.toString(), as I already said). You can tear that
apart if you like.
That said, l suggest we stop discussing about abstract software development
principles and get back to the concrete topic.
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14006027#comment-14006027
]
Jochen Kemnade commented on TAP5-2332:
--
Sorry for bringing new crazy ideas into the discussion still, but if we created
a LazyConcatenated class that implemented CharSequence and used CharSequence in
the signature of relevant methods (like the descriptions in OperationTracker),
we'd have to concatenate the strings only if we actually need the concatenated
result (in case of an exception or debug logging). And we wouldn't even break
backward compatibility... Alright, just kidding. Or am I...? :-D
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14006057#comment-14006057
]
Jochen Kemnade commented on TAP5-2332:
--
Please, let us stop discussing this. We just have different opinions. Lance and
Thiago don't want to avoid introducing more complexity and Michael and I want
to make the code faster.
Let's try to meet somewhere in between. I think, we are not too far apart
anyway. I understand that Lance and Thiago are not opposed to introducing the
concat method, nor to changing *some* of the existing code to use it.
Please give the some time to create a patch, afterward, we can go into a more
detailed discussion about that.
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14006236#comment-14006236
]
Thiago H. de Paula Figueiredo commented on TAP5-2332:
-
Jochen, you got it wrong. I and Lance do want to make the code faster. We've
stated this many times already. The difference is that we only want to make
changes (specifically replacing String.format() by InternalUtils.concat()) in
places which will actually make a difference in performance. That's not
abstract software development principles. This is a very concrete,
battle-tested principle, used for *decades* in all kinds of projects, large and
small. It's also about not wasting time changing code that won't make an
improvement in performance.
Michael, it cannot be applied to all places because there are some places
(maybe many) which aren't just simple string concatenation: they actually use
String.format() features string concatenation doesn't have, such as formatting
of numbers and dates, padded strings and non-sequential parameters. The whole
internationalization of messages in Tapestry needs that.
Guys, could you please listen a bit to people who are committers for many years
already? :-)
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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[
https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14006379#comment-14006379
]
Jochen Kemnade commented on TAP5-2332:
--
I've attached a new patch. The first simple benchmarks against test/app1/ with
ab show ~180 reqs/s with vanilla, ~215 reqs/s with patched version.
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-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)
