changeset 155:a094fc010120

8011095: PropertyHashMap.rehash() does not grow enough Reviewed-by: hannesw, lagergren Contributed-by: james.laskey@oracle.com
author jlaskey
date Sun, 31 Mar 2013 08:19:11 -0300
parents 704f3083af8a
children 3e4369fb810b af6fc67aa73d
files src/jdk/nashorn/internal/runtime/PropertyHashMap.java src/jdk/nashorn/internal/runtime/PropertyMap.java
diffstat 2 files changed, 20 insertions(+), 28 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk/nashorn/internal/runtime/PropertyHashMap.java	Fri Mar 29 18:38:27 2013 +0530
+++ b/src/jdk/nashorn/internal/runtime/PropertyHashMap.java	Sun Mar 31 08:19:11 2013 -0300
@@ -104,10 +104,10 @@
  */
 public final class PropertyHashMap implements Map <String, Property> {
     /** Number of initial bins. Power of 2. */
-    private static final int INITIAL_BINS = 16;
+    private static final int INITIAL_BINS = 32;
 
     /** Threshold before using bins. */
-    private static final int LIST_THRESHOLD = 4;
+    private static final int LIST_THRESHOLD = 8;
 
     /** Initial map. */
     public static final PropertyHashMap EMPTY_MAP = new PropertyHashMap();
@@ -300,8 +300,8 @@
      * @return Number of bins required.
      */
     private static int binsNeeded(final int n) {
-        // Allow for 25% padding.
-        return 1 << (32 - Integer.numberOfLeadingZeros((n + oneQuarter(n)) | (INITIAL_BINS - 1)));
+        // 50% padding
+        return 1 << (32 - Integer.numberOfLeadingZeros((n + (n >>> 1)) | (INITIAL_BINS - 1)));
     }
 
     /**
@@ -316,27 +316,15 @@
     }
 
     /**
-     * Used to calculate the current capacity of the bins.
-     *
-     * @param n Number of bin slots.
-     *
-     * @return 25% of n.
-     */
-    private static int oneQuarter(final int n) {
-        return n >>> 2;
-    }
-
-    /**
      * Regenerate the bin table after changing the number of bins.
      *
      * @param list    // List of all properties.
-     * @param newSize // New size of {@link PropertyHashMap}.
+     * @param binSize // New size of bins.
      *
      * @return Populated bins.
      */
-    private static Element[] rehash(final Element list, final int newSize) {
-        final int binsNeeded = binsNeeded(newSize);
-        final Element[] newBins = new Element[binsNeeded];
+    private static Element[] rehash(final Element list, final int binSize) {
+        final Element[] newBins = new Element[binSize];
         for (Element element = list; element != null; element = element.getLink()) {
             final Property property = element.getProperty();
             final String key = property.getKey();
@@ -390,7 +378,7 @@
         if (bins == null && newSize <= LIST_THRESHOLD) {
             newBins = null;
         } else if (newSize > threshold) {
-            newBins = rehash(list, newSize);
+            newBins = rehash(list, binsNeeded(newSize));
         } else {
             newBins = bins.clone();
         }
--- a/src/jdk/nashorn/internal/runtime/PropertyMap.java	Fri Mar 29 18:38:27 2013 +0530
+++ b/src/jdk/nashorn/internal/runtime/PropertyMap.java	Sun Mar 31 08:19:11 2013 -0300
@@ -526,11 +526,13 @@
      * @param newMap   {@link PropertyMap} associated with prototype.
      */
     private void addToProtoHistory(final ScriptObject newProto, final PropertyMap newMap) {
-        if (protoHistory == null) {
-            protoHistory = new WeakHashMap<>();
+        if (!properties.isEmpty()) {
+            if (protoHistory == null) {
+                protoHistory = new WeakHashMap<>();
+            }
+
+            protoHistory.put(newProto, new WeakReference<>(newMap));
         }
-
-        protoHistory.put(newProto, new WeakReference<>(newMap));
     }
 
     /**
@@ -540,11 +542,13 @@
      * @param newMap   Modified {@link PropertyMap}.
      */
     private void addToHistory(final Property property, final PropertyMap newMap) {
-        if (history == null) {
-            history = new LinkedHashMap<>();
+        if (!properties.isEmpty()) {
+            if (history == null) {
+                history = new LinkedHashMap<>();
+            }
+
+            history.put(property, newMap);
         }
-
-        history.put(property, newMap);
     }
 
     /**