[svn:parrot] r38388 - trunk/src/pmc

chromatic at svn.parrot.org chromatic at svn.parrot.org
Tue Apr 28 08:49:35 UTC 2009


Author: chromatic
Date: Tue Apr 28 08:49:34 2009
New Revision: 38388
URL: https://trac.parrot.org/parrot/changeset/38388

Log:
[PMC] Optimized keyed lookup slightly.  Hash get, exists, and delete operations
can use read-only Key STRINGs, as they do not modify the STRING.  Hash store
operations do store the Key STRING, so they need COW STRINGs (RT #60128).  The
Hash PMC has a new static function to make a read-only STRING from a Key.  This
optimization improves PGE performance by around 3.5%, in my rough benchmarks.

Modified:
   trunk/src/pmc/hash.pmc

Modified: trunk/src/pmc/hash.pmc
==============================================================================
--- trunk/src/pmc/hash.pmc	Tue Apr 28 08:16:26 2009	(r38387)
+++ trunk/src/pmc/hash.pmc	Tue Apr 28 08:49:34 2009	(r38388)
@@ -71,31 +71,68 @@
 
 /*
 
-=item C<static STRING *make_hash_key(PARROT_INTERP, PMC *key)>
+=item C<static STRING *make_ro_hash_key(PARROT_INTERP, PMC *key)>
+
+Returns a Parrot STRING for C<*key>.
 
-Returns a Parrot string for C<*key>.
+You I<must not> modify this STRING, nor pass it to anything which may store it.
+It's only safe to use for looking up elements of a hash or deleting them --
+I<never> storing them.  (If you have to ask why, don't use this function.  It's
+for optimization purposes only.)
 
 =cut
 
 */
 
+static STRING
+*make_ro_hash_key(PARROT_INTERP, NOTNULL(PMC *key))
+{
+    STRING *s;
+
+    switch (PObj_get_FLAGS(key) & KEY_type_FLAGS) {
+        case KEY_string_FLAG:
+            GETATTR_Key_str_key(interp, key, s);
+            break;
+        case KEY_string_FLAG | KEY_register_FLAG:
+        {
+            INTVAL  int_key;
+            GETATTR_Key_int_key(interp, key, int_key);
+            s = REG_STR(interp, int_key);
+            break;
+        }
+        default:
+            s = key_string(interp, key);
+    }
+
+    if (STRING_IS_NULL(s))
+        Parrot_ex_throw_from_c_args(interp, NULL,
+            EXCEPTION_UNEXPECTED_NULL, "Hash: Cannot use NULL STRING key");
+
+    return s;
+}
+
+
+/*
+
+=item C<static STRING *make_hash_key(PARROT_INTERP, PMC *key)>
+
+Returns a Parrot STRING for C<*key>.  This STRING is safe to modify or store.
+
+=cut
+
+*/
 
 PARROT_CANNOT_RETURN_NULL
 static STRING
 *make_hash_key(PARROT_INTERP, NOTNULL(PMC *key))
 {
-    if (!key)
-        Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_UNEXPECTED_NULL,
-            "Hash: Cannot use NULL key");
-    else {
-        STRING * const keystr = key_string(interp, key);
-        if (STRING_IS_NULL(keystr))
-            Parrot_ex_throw_from_c_args(interp, NULL,
-                EXCEPTION_UNEXPECTED_NULL, "Hash: Cannot use NULL STRING key");
-        else
-            return keystr;
-    }
+    STRING * const keystr = key_string(interp, key);
+
+    if (STRING_IS_NULL(keystr))
+        Parrot_ex_throw_from_c_args(interp, NULL,
+            EXCEPTION_UNEXPECTED_NULL, "Hash: Cannot use NULL STRING key");
 
+    return keystr;
 }
 
 /* Needs ext struct for the next_for_GC pointer
@@ -281,7 +318,7 @@
             }
         }
 
-        keystr  = make_hash_key(INTERP, key);
+        keystr  = make_ro_hash_key(INTERP, key);
         b       = parrot_hash_get_bucket(INTERP, hash, keystr);
 
         if (!b)
@@ -349,7 +386,7 @@
     VTABLE FLOATVAL get_number_keyed(PMC *key) {
         PMC               *nextkey;
         PMC               *valpmc;
-        STRING     * const keystr = make_hash_key(INTERP, key);
+        STRING     * const keystr = make_ro_hash_key(INTERP, key);
         HashBucket * const b      = parrot_hash_get_bucket(INTERP,
                                            (Hash *)SELF.get_pointer(), keystr);
 
@@ -479,7 +516,7 @@
             return (STRING *)parrot_hash_get_idx(INTERP, hash, key);
         }
 
-        keystr = make_hash_key(INTERP, key);
+        keystr = make_ro_hash_key(INTERP, key);
         b      = parrot_hash_get_bucket(INTERP, hash, keystr);
 
         if (!b)
@@ -606,7 +643,7 @@
             return result;
         }
 
-        keystr = make_hash_key(INTERP, key);
+        keystr = make_ro_hash_key(INTERP, key);
         b      = parrot_hash_get_bucket(INTERP, hash, keystr);
 
         if (!b)
@@ -920,16 +957,15 @@
 
     VTABLE INTVAL exists_keyed(PMC *key) {
         Hash   * const h  = (Hash *)SELF.get_pointer();
-        STRING * const sx = key_string(INTERP, key);
-        HashBucket    *b;
-
-        key = key_next(INTERP, key);
-        b   = parrot_hash_get_bucket(INTERP, h, sx);
+        STRING * const sx = make_ro_hash_key(INTERP, key);
+        HashBucket    *b  = parrot_hash_get_bucket(INTERP, h, sx);
 
         /* no such key */
         if (!b)
             return 0;
 
+        key = key_next(INTERP, key);
+
         /* lookup stops here */
         if (!key)
             return 1;
@@ -968,20 +1004,19 @@
 
     VTABLE INTVAL defined_keyed(PMC *key) {
         Hash   * const h  = (Hash *)SELF.get_pointer();
-        STRING * const sx = key_string(INTERP, key);
-        HashBucket    *b;
-
-        key = key_next(INTERP, key);
-        b   = parrot_hash_get_bucket(INTERP, h, sx);
+        STRING * const sx = make_ro_hash_key(INTERP, key);
+        HashBucket    *b  = parrot_hash_get_bucket(INTERP, h, sx);
 
         /* no such key */
         if (!b)
             return 0;
 
+        key = key_next(INTERP, key);
+
         if (!key)
-            return VTABLE_defined(INTERP, (PMC*)b->value);
+            return VTABLE_defined(INTERP, (PMC *)b->value);
         else
-            return VTABLE_defined_keyed(INTERP, (PMC*)b->value, key);
+            return VTABLE_defined_keyed(INTERP, (PMC *)b->value, key);
     }
 
 /*
@@ -1008,20 +1043,19 @@
 
     VTABLE void delete_keyed(PMC *key) {
         Hash   * const h  = (Hash *)SELF.get_pointer();
-        STRING * const sx = key_string(INTERP, key);
-        HashBucket    *b;
-
-        key = key_next(INTERP, key);
-        b   = parrot_hash_get_bucket(INTERP, h, sx);
+        STRING * const sx = make_ro_hash_key(INTERP, key);
+        HashBucket    *b  = parrot_hash_get_bucket(INTERP, h, sx);
 
         /* no such key */
         if (!b)
             return;
 
+        key = key_next(INTERP, key);
+
         if (!key)
             parrot_hash_delete(INTERP, h, sx);
         else
-            VTABLE_delete_keyed(INTERP, (PMC*)b->value, key);
+            VTABLE_delete_keyed(INTERP, (PMC *)b->value, key);
     }
 
 /*


More information about the parrot-commits mailing list