[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[ https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=14128205#comment-14128205 ] Jochen Kemnade commented on TAP5-2332: -- {qoute} ... but I really hope that just because you've found that String.format(...) is slower than concatenation you don't stop using it {quote} Sure I will, at least for concatenation. Concatenation with {{+}} is more readable *and* faster. I still prefer explicit {{StringBuilder}} usage though. > 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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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 Callable instead of Supplier 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-tabpanel&focusedCommentId=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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[ https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[ https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=14005885#comment-14005885 ] Lance commented on TAP5-2332: - I agree with Thiago. the patch patch changes the following... {code} .../org/apache/tapestry5/internal/OptionModelImpl.java | 2 +- .../beaneditor/MessagesConstraintGenerator.java| 3 +-- .../internal/services/DocumentLinkerImpl.java | 2 +- .../apache/tapestry5/internal/services/EventImpl.java | 2 +- .../tapestry5/internal/services/RenderQueueImpl.java | 18 ++ .../javascript/JavaScriptStackPathConstructorImpl.java | 4 +--- .../internal/structure/ComponentPageElementImpl.java | 2 +- .../internal/services/RenderQueueImplTest.java | 2 +- .../internal/services/PerThreadServiceLifecycle.java | 2 +- {code} Where is the evidence that these classes are hot-spots? > 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-tabpanel&focusedCommentId=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-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)
[jira] [Commented] (TAP5-2332) Optimize String concatenation performance
[ https://issues.apache.org/jira/browse/TAP5-2332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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-tabpanel&focusedCommentId=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)