You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by pa...@apache.org on 2016/06/08 20:28:33 UTC
[1/3] [lang] LANG-1229: Performance regression due to cyclic hashCode
guard
Repository: commons-lang
Updated Branches:
refs/heads/master 528f6e8e7 -> f08c4f6ae
LANG-1229: Performance regression due to cyclic hashCode guard
To fix this issues revert the unreleased "LANG-456: HashCodeBuilder throws StackOverflowError in bidirectional navigable".
This reverts commit b5749b4f54b30c0c2050e456c12cfcf516434f13.
Project: http://git-wip-us.apache.org/repos/asf/commons-lang/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-lang/commit/9bd439b4
Tree: http://git-wip-us.apache.org/repos/asf/commons-lang/tree/9bd439b4
Diff: http://git-wip-us.apache.org/repos/asf/commons-lang/diff/9bd439b4
Branch: refs/heads/master
Commit: 9bd439b4e0aa69050ef1baa537e552fa4620e5d4
Parents: 528f6e8
Author: pascalschumacher <pa...@gmx.net>
Authored: Wed Jun 8 22:21:41 2016 +0200
Committer: pascalschumacher <pa...@gmx.net>
Committed: Wed Jun 8 22:21:41 2016 +0200
----------------------------------------------------------------------
src/changes/changes.xml | 1 -
.../commons/lang3/builder/HashCodeBuilder.java | 30 +---------
.../lang3/builder/HashCodeBuilderTest.java | 59 +-------------------
3 files changed, 2 insertions(+), 88 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-lang/blob/9bd439b4/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index a49b1a0..a138162 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -117,7 +117,6 @@ The <action> type attribute can be add,update,fix,remove.
<action issue="LANG-1031" type="add" dev="britter" due-to="Felipe Adorno">Add annotations to exclude fields from ReflectionEqualsBuilder, ReflectionToStringBuilder and ReflectionHashCodeBuilder</action>
<action issue="LANG-1127" type="add" dev="chas, britter">Use JUnit rules to set and reset the default Locale and TimeZone</action>
<action issue="LANG-1128" type="fix" dev="britter" due-to="Jack Tan">JsonToStringStyle doesn't handle chars and objects correctly</action>
- <action issue="LANG-456" type="fix" dev="britter" due-to="Bob Fields, Woosan Ko, Bruno P. Kinoshita">HashCodeBuilder throws StackOverflowError in bidirectional navigable association</action>
<action issue="LANG-1126" type="fix" dev="britter">DateFormatUtilsTest.testSMTP depends on the default Locale</action>
<action issue="LANG-1123" type="fix" dev="chas" due-to="Christian P. Momon">Unit test FastDatePrinterTimeZonesTest needs a timezone set</action>
<action issue="LANG-916" type="fix" dev="chas" due-to="Christian P. Momon">CLONE - DateFormatUtils.format does not correctly change Calendar TimeZone in certain situations</action>
http://git-wip-us.apache.org/repos/asf/commons-lang/blob/9bd439b4/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java
index 72f4ded..70ca945 100644
--- a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java
+++ b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java
@@ -871,35 +871,7 @@ public class HashCodeBuilder implements Builder<Integer> {
append((Object[]) object);
}
} else {
- if (object instanceof Long) {
- append(((Long) object).longValue());
- } else if (object instanceof Integer) {
- append(((Integer) object).intValue());
- } else if (object instanceof Short) {
- append(((Short) object).shortValue());
- } else if (object instanceof Character) {
- append(((Character) object).charValue());
- } else if (object instanceof Byte) {
- append(((Byte) object).byteValue());
- } else if (object instanceof Double) {
- append(((Double) object).doubleValue());
- } else if (object instanceof Float) {
- append(((Float) object).floatValue());
- } else if (object instanceof Boolean) {
- append(((Boolean) object).booleanValue());
- } else if (object instanceof String) {
- iTotal = iTotal * iConstant + object.hashCode();
- } else {
- if (isRegistered(object)) {
- return this;
- }
- try {
- register(object);
- iTotal = iTotal * iConstant + object.hashCode();
- } finally {
- unregister(object);
- }
- }
+ iTotal = iTotal * iConstant + object.hashCode();
}
}
return this;
http://git-wip-us.apache.org/repos/asf/commons-lang/blob/9bd439b4/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java b/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java
index f33bf42..b268546 100644
--- a/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java
+++ b/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java
@@ -19,7 +19,6 @@ package org.apache.commons.lang3.builder;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
-
import org.junit.Test;
/**
@@ -31,8 +30,6 @@ public class HashCodeBuilderTest {
* A reflection test fixture.
*/
static class ReflectionTestCycleA {
- int index = 10;
- String name = "ReflectionTestCycleA";
ReflectionTestCycleB b;
@Override
@@ -45,8 +42,6 @@ public class HashCodeBuilderTest {
* A reflection test fixture.
*/
static class ReflectionTestCycleB {
- int index = 11;
- String name = "ReflectionTestCycleB";
ReflectionTestCycleA a;
@Override
@@ -55,42 +50,6 @@ public class HashCodeBuilderTest {
}
}
- /**
- * A nonreflection test fixture.
- */
- static class NonreflectionTestCycleA {
- int index = 20;
- String name = "NonreflectionTestCycleA";
- NonreflectionTestCycleB b;
-
- @Override
- public int hashCode() {
- HashCodeBuilder builder = new HashCodeBuilder();
- builder.append(index);
- builder.append(name);
- builder.append(b);
- return builder.toHashCode();
- }
- }
-
- /**
- * A nonreflection test fixture.
- */
- static class NonreflectionTestCycleB {
- int index = 21;
- String name = "NonreflectionTestCycleB";
- NonreflectionTestCycleA a;
-
- @Override
- public int hashCode() {
- HashCodeBuilder builder = new HashCodeBuilder();
- builder.append(index);
- builder.append(name);
- builder.append(a);
- return builder.toHashCode();
- }
- }
-
// -----------------------------------------------------------------------
@Test(expected=IllegalArgumentException.class)
@@ -562,7 +521,7 @@ public class HashCodeBuilderTest {
}
/**
- * Test Objects pointing to each other when {@link HashCodeBuilder#reflectionHashCode(Object, String...)} used.
+ * Test Objects pointing to each other.
*/
@Test
public void testReflectionObjectCycle() {
@@ -595,22 +554,6 @@ public class HashCodeBuilderTest {
}
/**
- * Test Objects pointing to each other when <code>append()</code> methods are used on <code>HashCodeBuilder</code> instance.
- */
- @Test
- public void testNonreflectionObjectCycle() {
- final NonreflectionTestCycleA a = new NonreflectionTestCycleA();
- final NonreflectionTestCycleB b = new NonreflectionTestCycleB();
- a.b = b;
- b.a = a;
-
- a.hashCode();
- assertNull(HashCodeBuilder.getRegistry());
- b.hashCode();
- assertNull(HashCodeBuilder.getRegistry());
- }
-
- /**
* Ensures LANG-520 remains true
*/
@Test
[2/3] [lang] LANG-1229: HashCodeBuilder.append(Object,
Object) is too big to be inlined,
which prevents whole builder to be scalarized (closes #142)
Posted by pa...@apache.org.
LANG-1229: HashCodeBuilder.append(Object,Object) is too big to be inlined, which prevents whole builder to be scalarized (closes #142)
Project: http://git-wip-us.apache.org/repos/asf/commons-lang/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-lang/commit/18b1bc20
Tree: http://git-wip-us.apache.org/repos/asf/commons-lang/tree/18b1bc20
Diff: http://git-wip-us.apache.org/repos/asf/commons-lang/diff/18b1bc20
Branch: refs/heads/master
Commit: 18b1bc203bdb236bfd9084193aa2d55d4b605139
Parents: 9bd439b
Author: Philippe Marschall <ph...@gmail.com>
Authored: Sun May 15 16:28:48 2016 +0200
Committer: pascalschumacher <pa...@gmx.net>
Committed: Wed Jun 8 22:27:58 2016 +0200
----------------------------------------------------------------------
.../commons/lang3/builder/HashCodeBuilder.java | 60 ++++++++++++--------
1 file changed, 37 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-lang/blob/18b1bc20/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java
index 70ca945..98ed5c5 100644
--- a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java
+++ b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java
@@ -847,29 +847,10 @@ public class HashCodeBuilder implements Builder<Integer> {
iTotal = iTotal * iConstant;
} else {
- if(object.getClass().isArray()) {
- // 'Switch' on type of array, to dispatch to the correct handler
- // This handles multi dimensional arrays
- if (object instanceof long[]) {
- append((long[]) object);
- } else if (object instanceof int[]) {
- append((int[]) object);
- } else if (object instanceof short[]) {
- append((short[]) object);
- } else if (object instanceof char[]) {
- append((char[]) object);
- } else if (object instanceof byte[]) {
- append((byte[]) object);
- } else if (object instanceof double[]) {
- append((double[]) object);
- } else if (object instanceof float[]) {
- append((float[]) object);
- } else if (object instanceof boolean[]) {
- append((boolean[]) object);
- } else {
- // Not an array of primitives
- append((Object[]) object);
- }
+ if (object.getClass().isArray()) {
+ // factor out array case in order to keep method small enough
+ // to be inlined
+ appendArray(object);
} else {
iTotal = iTotal * iConstant + object.hashCode();
}
@@ -879,6 +860,39 @@ public class HashCodeBuilder implements Builder<Integer> {
/**
* <p>
+ * Append a <code>hashCode</code> for an array.
+ * </p>
+ *
+ * @param object
+ * the array to add to the <code>hashCode</code>
+ */
+ private void appendArray(final Object object) {
+ // 'Switch' on type of array, to dispatch to the correct handler
+ // This handles multi dimensional arrays
+ if (object instanceof long[]) {
+ append((long[]) object);
+ } else if (object instanceof int[]) {
+ append((int[]) object);
+ } else if (object instanceof short[]) {
+ append((short[]) object);
+ } else if (object instanceof char[]) {
+ append((char[]) object);
+ } else if (object instanceof byte[]) {
+ append((byte[]) object);
+ } else if (object instanceof double[]) {
+ append((double[]) object);
+ } else if (object instanceof float[]) {
+ append((float[]) object);
+ } else if (object instanceof boolean[]) {
+ append((boolean[]) object);
+ } else {
+ // Not an array of primitives
+ append((Object[]) object);
+ }
+ }
+
+ /**
+ * <p>
* Append a <code>hashCode</code> for an <code>Object</code> array.
* </p>
*
[3/3] [lang] LANG-1229: add changes.xml entry
Posted by pa...@apache.org.
LANG-1229: add changes.xml entry
Project: http://git-wip-us.apache.org/repos/asf/commons-lang/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-lang/commit/f08c4f6a
Tree: http://git-wip-us.apache.org/repos/asf/commons-lang/tree/f08c4f6a
Diff: http://git-wip-us.apache.org/repos/asf/commons-lang/diff/f08c4f6a
Branch: refs/heads/master
Commit: f08c4f6ae9f8190bcaf0a8bbb82530233bf51bf9
Parents: 18b1bc2
Author: pascalschumacher <pa...@gmx.net>
Authored: Wed Jun 8 22:28:19 2016 +0200
Committer: pascalschumacher <pa...@gmx.net>
Committed: Wed Jun 8 22:28:19 2016 +0200
----------------------------------------------------------------------
src/changes/changes.xml | 1 +
1 file changed, 1 insertion(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-lang/blob/f08c4f6a/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index a138162..e48726d 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -46,6 +46,7 @@ The <action> type attribute can be add,update,fix,remove.
<body>
<release version="3.5" date="tba" description="tba">
+ <action issue="LANG-1229" type="update" dev="pschumacher" due-to="Ruslan Cheremin">HashCodeBuilder.append(Object,Object) is too big to be inlined, which prevents whole builder to be scalarized</action>
<action issue="LANG-1085" type="add" dev="oheger" due-to="oheger / kinow">Add a circuit breaker implementation</action>
<action issue="LANG-1013" type="add" dev="pschumacher" due-to="Thiago Andrade">Add StringUtils.truncate()</action>
<action issue="LANG-1195" type="add" dev="pschumacher" due-to="Derek C. Ashmore">Enhance MethodUtils to allow invocation of private methods</action>