Re: [cp-patches] [PATCH] Bump up HashMap default initial capacity

[Dr Andrew John Hughes]

On 16:07 Tue 22 Feb , Mark Wielaard wrote:
> On Tue, 2011-02-22 at 16:58 +0200, Pekka Enberg wrote:
> > While debugging Jython bootstrap issues with GNU Classpath, I noticed that
> > HashMap.DEFAULT_CAPACITY is set to 11 although 1.4 Javadocs, for example,
> > 
> > define it to be 16.  Signed-off-by: Pekka Enberg 
> >/**
> > -   * Default number of buckets. This is the value the JDK 1.3 uses. Some
> > -   * early documentation specified this value as 101. That is incorrect.
> > +   * Default number of buckets; this is explicitly specified by the spec.
> > * Package visible for use by HashSet.
> > */
> > -  static final int DEFAULT_CAPACITY = 11;
> > +  static final int DEFAULT_CAPACITY = 16;
> 
> Just say "this is currently set to 16."
> Since obviously "the spec" changed a bit over time.
> 

I agree with this.  Not only does the spec. change, but then the user has
to work out what 'the spec' refers to and go check it.  Much easier to
just say 'currently set to 16'.

> The actual change seems OK.
> 
> Thanks,
> 
> Mark
> 
> 

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37

Re: [cp-patches] [PATCH] Fix HashMap.put() to check for hashCode equality before equals()

[Mark Wielaard]

On Tue, 2011-02-22 at 17:02 +0200, Pekka Enberg wrote:
> This patch is needed to run Jython 2.5.2 RC 4 under JamVM and GNU Classpath 
> CVS
> HEAD. It turns out Jythin bootstrap is bit hairy and assumes HashMap.put()
> checks for hashCode equality before invoking Object.equals().
> [...]
>  2011-02-22  Pekka Enberg  
>  
>   * java/util/HashMap:
> + (put): Check for key hashCode equality before invoking
> + Object.equals() to fix compatibility issue with Jython.

That seems a fine optimization in itself. But any application depending
on it is pretty e :{ O well, that is how it is I guess. We had
something similar in the initialization order of eclipse, where they
depended on equals being called in a particular direction. sigh.

Thanks,

Mark

Re: [cp-patches] [PATCH] Bump up HashMap default initial capacity

[Mark Wielaard]

On Tue, 2011-02-22 at 16:58 +0200, Pekka Enberg wrote:
> While debugging Jython bootstrap issues with GNU Classpath, I noticed that
> HashMap.DEFAULT_CAPACITY is set to 11 although 1.4 Javadocs, for example,
> 
> define it to be 16.  Signed-off-by: Pekka Enberg 
>/**
> -   * Default number of buckets. This is the value the JDK 1.3 uses. Some
> -   * early documentation specified this value as 101. That is incorrect.
> +   * Default number of buckets; this is explicitly specified by the spec.
> * Package visible for use by HashSet.
> */
> -  static final int DEFAULT_CAPACITY = 11;
> +  static final int DEFAULT_CAPACITY = 16;

Just say "this is currently set to 16."
Since obviously "the spec" changed a bit over time.

The actual change seems OK.

Thanks,

Mark

[cp-patches] [PATCH] Fix HashMap.put() to check for hashCode equality before equals()

[Pekka Enberg]

This patch is needed to run Jython 2.5.2 RC 4 under JamVM and GNU Classpath CVS
HEAD. It turns out Jythin bootstrap is bit hairy and assumes HashMap.put()
checks for hashCode equality before invoking Object.equals().

Signed-off-by: Pekka Enberg 
---
 ChangeLog  |6 ++
 java/util/HashMap.java |2 +-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7eeacf0..05aa794 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,12 @@
 2011-02-22  Pekka Enberg  
 
* java/util/HashMap:
+   (put): Check for key hashCode equality before invoking
+   Object.equals() to fix compatibility issue with Jython.
+
+2011-02-22  Pekka Enberg  
+
+   * java/util/HashMap:
(DEFAULT_CAPACITY): Make default initial capacity 16 as it is
defined in official Javadocs.
 
diff --git a/java/util/HashMap.java b/java/util/HashMap.java
index 108bf25..2f56932 100644
--- a/java/util/HashMap.java
+++ b/java/util/HashMap.java
@@ -345,7 +345,7 @@ public class HashMap extends AbstractMap
 
 while (e != null)
   {
-if (equals(key, e.key))
+if ((key.hashCode() == e.key.hashCode()) && equals(key, e.key))
   {
 e.access(); // Must call this for bookkeeping in LinkedHashMap.
 V r = e.value;
-- 
1.7.1

[cp-patches] [PATCH] Bump up HashMap default initial capacity

[Pekka Enberg]

While debugging Jython bootstrap issues with GNU Classpath, I noticed that
HashMap.DEFAULT_CAPACITY is set to 11 although 1.4 Javadocs, for example,

define it to be 16.  Signed-off-by: Pekka Enberg 
---
 ChangeLog  |6 ++
 java/util/HashMap.java |5 ++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f1c7c31..7eeacf0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-02-22  Pekka Enberg  
+
+   * java/util/HashMap:
+   (DEFAULT_CAPACITY): Make default initial capacity 16 as it is
+   defined in official Javadocs.
+
 2010-02-16  Pekka Enberg  
 
* java/util/Formatter.java:
diff --git a/java/util/HashMap.java b/java/util/HashMap.java
index 55d81c6..108bf25 100644
--- a/java/util/HashMap.java
+++ b/java/util/HashMap.java
@@ -100,11 +100,10 @@ public class HashMap extends AbstractMap
   implements Map, Cloneable, Serializable
 {
   /**
-   * Default number of buckets. This is the value the JDK 1.3 uses. Some
-   * early documentation specified this value as 101. That is incorrect.
+   * Default number of buckets; this is explicitly specified by the spec.
* Package visible for use by HashSet.
*/
-  static final int DEFAULT_CAPACITY = 11;
+  static final int DEFAULT_CAPACITY = 16;
 
   /**
* The default load factor; this is explicitly specified by the spec.
-- 
1.7.1