From 9bd439b4e0aa69050ef1baa537e552fa4620e5d4 Mon Sep 17 00:00:00 2001 From: pascalschumacher Date: Wed, 8 Jun 2016 22:21:41 +0200 Subject: [PATCH] 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. --- src/changes/changes.xml | 1 - .../lang3/builder/HashCodeBuilder.java | 30 +--------- .../lang3/builder/HashCodeBuilderTest.java | 59 +------------------ 3 files changed, 2 insertions(+), 88 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index a49b1a000..a1381627c 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -117,7 +117,6 @@ The type attribute can be add,update,fix,remove. Add annotations to exclude fields from ReflectionEqualsBuilder, ReflectionToStringBuilder and ReflectionHashCodeBuilder Use JUnit rules to set and reset the default Locale and TimeZone JsonToStringStyle doesn't handle chars and objects correctly - HashCodeBuilder throws StackOverflowError in bidirectional navigable association DateFormatUtilsTest.testSMTP depends on the default Locale Unit test FastDatePrinterTimeZonesTest needs a timezone set CLONE - DateFormatUtils.format does not correctly change Calendar TimeZone in certain situations 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 72f4dedbb..70ca9454a 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 HashCodeBuilder append(final Object object) { 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; 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 f33bf42de..b26854682 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 @@ 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 int hashCode() { * A reflection test fixture. */ static class ReflectionTestCycleB { - int index = 11; - String name = "ReflectionTestCycleB"; ReflectionTestCycleA a; @Override @@ -55,42 +50,6 @@ public int hashCode() { } } - /** - * 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 TestObjectWithMultipleFields(final int one, final int two, final int thre } /** - * Test Objects pointing to each other when {@link HashCodeBuilder#reflectionHashCode(Object, String...)} used. + * Test Objects pointing to each other. */ @Test public void testReflectionObjectCycle() { @@ -594,22 +553,6 @@ public void testReflectionObjectCycle() { assertNull(HashCodeBuilder.getRegistry()); } - /** - * Test Objects pointing to each other when append() methods are used on HashCodeBuilder 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 */