[jira] [Commented] (LANG-1229) Performance regression due to cyclic hashCode guard
[ https://issues.apache.org/jira/browse/LANG-1229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15321391#comment-15321391 ] Pascal Schumacher commented on LANG-1229: - Pull request applied. Thanks. I reopened LANG-456. > Performance regression due to cyclic hashCode guard > --- > > Key: LANG-1229 > URL: https://issues.apache.org/jira/browse/LANG-1229 > Project: Commons Lang > Issue Type: Bug > Components: lang.builder.* >Affects Versions: 3.5 >Reporter: Philippe Marschall > > We observed a severe performance regression in HashCodeBuilder in 3.5 trunk > compared to 3.4 release. We get about 20% of the throughput in 3.5 trunk for > common cases compared to 3.4 release. Previously there was no noticeable > overhead of using HashCodeBuilder. Investigating we found the performance > degradation was caused by the fix for LANG-456. It causes the method to be > too large to be inlined and escape analysis to fail (see LANG-1218 for a > related discussion). > We currently do not see a way to keep the 3.4 performance and support cyclic > graphs. The append methods have not supported cycles for so long we feel it's > de facto part of the contract by now. Since neither the JDK nor the Guava > hashCode helper methods support cyclic graphs we don't believe this is an > unreasonable assumption. In addition EqualsBuilder#append(Object,Object) > doesn't support cycles. > If supporting cycles is a requirement we propose the introduction of new > #appendRecursive (or named differently) methods to both HashCodeBuilder and > EqualsBuilder that add cycle guards. If that is an acceptable compromise we > would be willing to provide patches. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1229) Performance regression due to cyclic hashCode guard
[ https://issues.apache.org/jira/browse/LANG-1229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15321385#comment-15321385 ] ASF GitHub Bot commented on LANG-1229: -- Github user PascalSchumacher commented on the issue: https://github.com/apache/commons-lang/pull/142 Thanks! :+1: > Performance regression due to cyclic hashCode guard > --- > > Key: LANG-1229 > URL: https://issues.apache.org/jira/browse/LANG-1229 > Project: Commons Lang > Issue Type: Bug > Components: lang.builder.* >Affects Versions: 3.5 >Reporter: Philippe Marschall > > We observed a severe performance regression in HashCodeBuilder in 3.5 trunk > compared to 3.4 release. We get about 20% of the throughput in 3.5 trunk for > common cases compared to 3.4 release. Previously there was no noticeable > overhead of using HashCodeBuilder. Investigating we found the performance > degradation was caused by the fix for LANG-456. It causes the method to be > too large to be inlined and escape analysis to fail (see LANG-1218 for a > related discussion). > We currently do not see a way to keep the 3.4 performance and support cyclic > graphs. The append methods have not supported cycles for so long we feel it's > de facto part of the contract by now. Since neither the JDK nor the Guava > hashCode helper methods support cyclic graphs we don't believe this is an > unreasonable assumption. In addition EqualsBuilder#append(Object,Object) > doesn't support cycles. > If supporting cycles is a requirement we propose the introduction of new > #appendRecursive (or named differently) methods to both HashCodeBuilder and > EqualsBuilder that add cycle guards. If that is an acceptable compromise we > would be willing to provide patches. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1229) Performance regression due to cyclic hashCode guard
[ https://issues.apache.org/jira/browse/LANG-1229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15321384#comment-15321384 ] ASF GitHub Bot commented on LANG-1229: -- Github user asfgit closed the pull request at: https://github.com/apache/commons-lang/pull/142 > Performance regression due to cyclic hashCode guard > --- > > Key: LANG-1229 > URL: https://issues.apache.org/jira/browse/LANG-1229 > Project: Commons Lang > Issue Type: Bug > Components: lang.builder.* >Affects Versions: 3.5 >Reporter: Philippe Marschall > > We observed a severe performance regression in HashCodeBuilder in 3.5 trunk > compared to 3.4 release. We get about 20% of the throughput in 3.5 trunk for > common cases compared to 3.4 release. Previously there was no noticeable > overhead of using HashCodeBuilder. Investigating we found the performance > degradation was caused by the fix for LANG-456. It causes the method to be > too large to be inlined and escape analysis to fail (see LANG-1218 for a > related discussion). > We currently do not see a way to keep the 3.4 performance and support cyclic > graphs. The append methods have not supported cycles for so long we feel it's > de facto part of the contract by now. Since neither the JDK nor the Guava > hashCode helper methods support cyclic graphs we don't believe this is an > unreasonable assumption. In addition EqualsBuilder#append(Object,Object) > doesn't support cycles. > If supporting cycles is a requirement we propose the introduction of new > #appendRecursive (or named differently) methods to both HashCodeBuilder and > EqualsBuilder that add cycle guards. If that is an acceptable compromise we > would be willing to provide patches. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1229) Performance regression due to cyclic hashCode guard
[ https://issues.apache.org/jira/browse/LANG-1229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15315913#comment-15315913 ] ASF GitHub Bot commented on LANG-1229: -- Github user PascalSchumacher commented on the issue: https://github.com/apache/commons-lang/pull/142 To keep the code clean the pull request should also undo the other additions to `HashCodeBuilderTest` done with https://github.com/apache/commons-lang/commit/b5749b4f54b30c0c2050e456c12cfcf516434f13 > Performance regression due to cyclic hashCode guard > --- > > Key: LANG-1229 > URL: https://issues.apache.org/jira/browse/LANG-1229 > Project: Commons Lang > Issue Type: Bug > Components: lang.builder.* >Affects Versions: 3.5 >Reporter: Philippe Marschall > > We observed a severe performance regression in HashCodeBuilder in 3.5 trunk > compared to 3.4 release. We get about 20% of the throughput in 3.5 trunk for > common cases compared to 3.4 release. Previously there was no noticeable > overhead of using HashCodeBuilder. Investigating we found the performance > degradation was caused by the fix for LANG-456. It causes the method to be > too large to be inlined and escape analysis to fail (see LANG-1218 for a > related discussion). > We currently do not see a way to keep the 3.4 performance and support cyclic > graphs. The append methods have not supported cycles for so long we feel it's > de facto part of the contract by now. Since neither the JDK nor the Guava > hashCode helper methods support cyclic graphs we don't believe this is an > unreasonable assumption. In addition EqualsBuilder#append(Object,Object) > doesn't support cycles. > If supporting cycles is a requirement we propose the introduction of new > #appendRecursive (or named differently) methods to both HashCodeBuilder and > EqualsBuilder that add cycle guards. If that is an acceptable compromise we > would be willing to provide patches. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1229) Performance regression due to cyclic hashCode guard
[ https://issues.apache.org/jira/browse/LANG-1229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15305278#comment-15305278 ] Philippe Marschall commented on LANG-1229: -- The patch is indeed missing the new method. I'm unsure what to name it. > Performance regression due to cyclic hashCode guard > --- > > Key: LANG-1229 > URL: https://issues.apache.org/jira/browse/LANG-1229 > Project: Commons Lang > Issue Type: Bug > Components: lang.builder.* >Affects Versions: 3.5 >Reporter: Philippe Marschall > > We observed a severe performance regression in HashCodeBuilder in 3.5 trunk > compared to 3.4 release. We get about 20% of the throughput in 3.5 trunk for > common cases compared to 3.4 release. Previously there was no noticeable > overhead of using HashCodeBuilder. Investigating we found the performance > degradation was caused by the fix for LANG-456. It causes the method to be > too large to be inlined and escape analysis to fail (see LANG-1218 for a > related discussion). > We currently do not see a way to keep the 3.4 performance and support cyclic > graphs. The append methods have not supported cycles for so long we feel it's > de facto part of the contract by now. Since neither the JDK nor the Guava > hashCode helper methods support cyclic graphs we don't believe this is an > unreasonable assumption. In addition EqualsBuilder#append(Object,Object) > doesn't support cycles. > If supporting cycles is a requirement we propose the introduction of new > #appendRecursive (or named differently) methods to both HashCodeBuilder and > EqualsBuilder that add cycle guards. If that is an acceptable compromise we > would be willing to provide patches. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1229) Performance regression due to cyclic hashCode guard
[ https://issues.apache.org/jira/browse/LANG-1229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304216#comment-15304216 ] Pascal Schumacher commented on LANG-1229: - The patch is missing the new method (maybe call it something similar to #appendWithCycleCheck?). > Performance regression due to cyclic hashCode guard > --- > > Key: LANG-1229 > URL: https://issues.apache.org/jira/browse/LANG-1229 > Project: Commons Lang > Issue Type: Bug > Components: lang.builder.* >Affects Versions: 3.5 >Reporter: Philippe Marschall > > We observed a severe performance regression in HashCodeBuilder in 3.5 trunk > compared to 3.4 release. We get about 20% of the throughput in 3.5 trunk for > common cases compared to 3.4 release. Previously there was no noticeable > overhead of using HashCodeBuilder. Investigating we found the performance > degradation was caused by the fix for LANG-456. It causes the method to be > too large to be inlined and escape analysis to fail (see LANG-1218 for a > related discussion). > We currently do not see a way to keep the 3.4 performance and support cyclic > graphs. The append methods have not supported cycles for so long we feel it's > de facto part of the contract by now. Since neither the JDK nor the Guava > hashCode helper methods support cyclic graphs we don't believe this is an > unreasonable assumption. In addition EqualsBuilder#append(Object,Object) > doesn't support cycles. > If supporting cycles is a requirement we propose the introduction of new > #appendRecursive (or named differently) methods to both HashCodeBuilder and > EqualsBuilder that add cycle guards. If that is an acceptable compromise we > would be willing to provide patches. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1229) Performance regression due to cyclic hashCode guard
[ https://issues.apache.org/jira/browse/LANG-1229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15284932#comment-15284932 ] Philippe Marschall commented on LANG-1229: -- A potential patch https://github.com/apache/commons-lang/pull/142 > Performance regression due to cyclic hashCode guard > --- > > Key: LANG-1229 > URL: https://issues.apache.org/jira/browse/LANG-1229 > Project: Commons Lang > Issue Type: Bug > Components: lang.builder.* >Affects Versions: 3.5 >Reporter: Philippe Marschall > > We observed a severe performance regression in HashCodeBuilder in 3.5 trunk > compared to 3.4 release. We get about 20% of the throughput in 3.5 trunk for > common cases compared to 3.4 release. Previously there was no noticeable > overhead of using HashCodeBuilder. Investigating we found the performance > degradation was caused by the fix for LANG-456. It causes the method to be > too large to be inlined and escape analysis to fail (see LANG-1218 for a > related discussion). > We currently do not see a way to keep the 3.4 performance and support cyclic > graphs. The append methods have not supported cycles for so long we feel it's > de facto part of the contract by now. Since neither the JDK nor the Guava > hashCode helper methods support cyclic graphs we don't believe this is an > unreasonable assumption. In addition EqualsBuilder#append(Object,Object) > doesn't support cycles. > If supporting cycles is a requirement we propose the introduction of new > #appendRecursive (or named differently) methods to both HashCodeBuilder and > EqualsBuilder that add cycle guards. If that is an acceptable compromise we > would be willing to provide patches. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1229) Performance regression due to cyclic hashCode guard
[ https://issues.apache.org/jira/browse/LANG-1229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15284263#comment-15284263 ] Philippe Marschall commented on LANG-1229: -- Personally I don't like boolean flags to methods that change the behaviour of methods. Personally I prefer two different methods for two different things. But if a boolean flag is an acceptable compromise then I'll take it. > Performance regression due to cyclic hashCode guard > --- > > Key: LANG-1229 > URL: https://issues.apache.org/jira/browse/LANG-1229 > Project: Commons Lang > Issue Type: Bug > Components: lang.builder.* >Affects Versions: 3.5 >Reporter: Philippe Marschall > > We observed a severe performance regression in HashCodeBuilder in 3.5 trunk > compared to 3.4 release. We get about 20% of the throughput in 3.5 trunk for > common cases compared to 3.4 release. Previously there was no noticeable > overhead of using HashCodeBuilder. Investigating we found the performance > degradation was caused by the fix for LANG-456. It causes the method to be > too large to be inlined and escape analysis to fail (see LANG-1218 for a > related discussion). > We currently do not see a way to keep the 3.4 performance and support cyclic > graphs. The append methods have not supported cycles for so long we feel it's > de facto part of the contract by now. Since neither the JDK nor the Guava > hashCode helper methods support cyclic graphs we don't believe this is an > unreasonable assumption. In addition EqualsBuilder#append(Object,Object) > doesn't support cycles. > If supporting cycles is a requirement we propose the introduction of new > #appendRecursive (or named differently) methods to both HashCodeBuilder and > EqualsBuilder that add cycle guards. If that is an acceptable compromise we > would be willing to provide patches. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1229) Performance regression due to cyclic hashCode guard
[ https://issues.apache.org/jira/browse/LANG-1229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15283890#comment-15283890 ] Gary Gregory commented on LANG-1229: Or maybe the new method is still called append but takes a boolean indicating support for cycles/recursion. > Performance regression due to cyclic hashCode guard > --- > > Key: LANG-1229 > URL: https://issues.apache.org/jira/browse/LANG-1229 > Project: Commons Lang > Issue Type: Bug > Components: lang.builder.* >Affects Versions: 3.5 >Reporter: Philippe Marschall > > We observed a severe performance regression in HashCodeBuilder in 3.5 trunk > compared to 3.4 release. We get about 20% of the throughput in 3.5 trunk for > common cases compared to 3.4 release. Previously there was no noticeable > overhead of using HashCodeBuilder. Investigating we found the performance > degradation was caused by the fix for LANG-456. It causes the method to be > too large to be inlined and escape analysis to fail (see LANG-1218 for a > related discussion). > We currently do not see a way to keep the 3.4 performance and support cyclic > graphs. The append methods have not supported cycles for so long we feel it's > de facto part of the contract by now. Since neither the JDK nor the Guava > hashCode helper methods support cyclic graphs we don't believe this is an > unreasonable assumption. In addition EqualsBuilder#append(Object,Object) > doesn't support cycles. > If supporting cycles is a requirement we propose the introduction of new > #appendRecursive (or named differently) methods to both HashCodeBuilder and > EqualsBuilder that add cycle guards. If that is an acceptable compromise we > would be willing to provide patches. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
