Clarifying PersistentByteMap (was: Re: [patch] Fix PR java/22189)

[Robin Green <greenrd at greenrd dot org>]

On Mon, Jun 27, 2005 at 10:40:15AM +0100, Andrew Haley wrote:
> Robin Green writes:
>  > Hi,
>  > 
>  > Below is a copy of my fix and cleanup for
>  > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22189
>  > 
>  > The actual fix is only one line, but I attempted to rename
>  > stuff to clarify the code. Please see the bug report for explanation.
>  > 
>  > Needs testing and applying.
> 
> Seems reasonable, but:
> 
>  - Mixes substantive changes and reformatting.

OK, I've posted the bug fix separately. This email deals with the field rename.

The purpose of renaming the capacity field to internal_capacity is
to clarify (for the benefit of newcomers reading the code) that there
are two measures of capacity with different values:

1. The "external_capacity", which is what clients of the PersistentByteMap
class see: the maximum number of elements that are allowed to be inserted.
The bug was that this external capacity changed from 1 to 0 due to a
rounding error.

2. The "internal_capacity", which is the actual number of slots in the
table, and is at least (external_capacity*3)/2.

It could be clearer, I know.

>  - Replaces correctly formatted code with differently formatted code
>    for no apparent reason;

I probably had mismatching tab/indent settings in Eclipse - hopefully this time
indentations have not been changed. 

> at one point a pair of braces is removed
>    and replaced by double-spaced blank lines.

I've reverted that unnecessary change.

>  - A new local variable, realCapacity, is introduced, but it doesn't
>    seem to do anything useful.

I've reverted that unnecessary change. I was trying to preserve the
optimisation of using a local variable rather than reading from an instance
variable several times (I've changed it to read from internal_capacity because
it is dealing with the internal capacity there, not the external capacity).

-- 
Robin

--- gnu/gcj/runtime/PersistentByteMap.java~	2005-06-27 12:38:18.000000000 +0100
+++ gnu/gcj/runtime/PersistentByteMap.java	2005-06-27 12:49:35.000000000 +0100
@@ -67,7 +67,7 @@
 
   static private final int MAGIC = 0;
   static private final int VERSION = 4;
-  static private final int CAPACITY = 8;
+  static private final int INTERNAL_CAPACITY = 8;
   static private final int TABLE_BASE = 12;
   static private final int STRING_BASE = 16;
   static private final int STRING_SIZE = 20;
@@ -78,7 +78,7 @@
 
   static private final int TABLE_ENTRY_SIZE = 2 * INT_SIZE;
 
-  private int capacity;   // real number of entries
+  private int internal_capacity;   // number of entries
   private int table_base;   // offset from start of file, in bytes
   private int string_base;  // offset from start of file, in bytes
   private int string_size;  // size of string table, in bytes
@@ -155,7 +155,7 @@
       throw new IllegalArgumentException(f.getName());
 
     table_base = getWord (TABLE_BASE);
-    capacity = getWord (CAPACITY);
+    internal_capacity = getWord (INTERNAL_CAPACITY);
     string_base = getWord (STRING_BASE);
     string_size = getWord (STRING_SIZE);
     file_size = getWord (FILE_SIZE);
@@ -187,11 +187,11 @@
       while (! size.isProbablePrime(10))
 	size = size.add(two);
       
-      this.capacity = capacity = size.intValue();
+      this.internal_capacity = size.intValue();
     }
 
     table_base = 64;
-    string_base = table_base + capacity * TABLE_ENTRY_SIZE;
+    string_base = table_base + internal_capacity * TABLE_ENTRY_SIZE;
     string_size = 0;
     file_size = string_base;
     elements = 0;
@@ -207,12 +207,12 @@
     fc = raf.getChannel();
     buf = fc.map(FileChannel.MapMode.READ_WRITE, 0, raf.length());
 
-    for (int i = 0; i < capacity; i++)
+    for (int i = 0; i < internal_capacity; i++)
       putKeyPos(UNUSED_ENTRY, i);
         
     putWord(0x67636a64, MAGIC);
     putWord(0x01, VERSION);
-    putWord(capacity, CAPACITY);
+    putWord(internal_capacity, INTERNAL_CAPACITY);
     putWord(table_base, TABLE_BASE);
     putWord(string_base, STRING_BASE);
     putWord(file_size, FILE_SIZE);
@@ -305,7 +305,7 @@
          + ((b[1]&0xffL)<<8) 
          + ((b[2]&0xffL)<<16) 
          + ((b[3]&0xffL)<<24));
-    long result = hashIndex % (long)capacity;
+    long result = hashIndex % (long)internal_capacity;
     return (int)result;
   }
         
@@ -326,7 +326,7 @@
         // not be theoretically as good as open addressing, but it has
         // good cache behviour.
         hashIndex++;
-        hashIndex %= capacity;
+        hashIndex %= internal_capacity;
       }
     while (true);
   }
@@ -360,7 +360,7 @@
           }
                 
         hashIndex++;
-        hashIndex %= capacity;
+        hashIndex %= internal_capacity;
       }
     while (true);
   }
@@ -376,7 +376,7 @@
 	  {
 	    values = new HashMap();
 	
-	    for (int i = 0; i < capacity; i++)
+	    for (int i = 0; i < internal_capacity; i++)
 	      if (getKeyPos(i) != UNUSED_ENTRY)
 		{
 		  int pos = getValuePos(i);
@@ -437,7 +437,7 @@
   {
     // With the the table 2/3 full there will be on average 2 probes
     // for a successful search and 5 probes for an unsuccessful one.
-    return capacity * 2/3;
+    return internal_capacity * 2/3;
   }
 
   public void force()
@@ -463,7 +463,7 @@
     throws IllegalAccessException
   {
     // We can use a fast copy if the size of a map has not changed.
-    if (this.elements == 0 && t.capacity == this.capacity
+    if (this.elements == 0 && t.internal_capacity == this.internal_capacity
 	&& t.length == this.length)
       {
 	this.buf.position(0);
@@ -528,7 +528,7 @@
     public Object next()
     {
       count--;
-      for (int i = idx; i < capacity; i++)
+      for (int i = idx; i < internal_capacity; i++)
         if (getKeyPos(i) != UNUSED_ENTRY)
           {
             idx = i+1;

Attachment: pgp00000.pgp
Description: PGP signature