commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pascalschumac...@apache.org
Subject [1/3] [lang] LANG-1229: Performance regression due to cyclic hashCode guard
Date Wed, 08 Jun 2016 20:28:33 GMT
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 <pascalschumacher@gmx.net>
Authored: Wed Jun 8 22:21:41 2016 +0200
Committer: pascalschumacher <pascalschumacher@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


Mime
View raw message