[fortran-dev, patch] Parsed format data caching

Daniel Kraft d@domob.eu
Sun Mar 29 18:45:00 GMT 2009


Hi Jerry,

Jerry DeLisle wrote:
> This patch uses the simplest of hashing functions to hash a format 
> string and save a pointer to the parsed format data and its tokenized 
> fnode tree.
> 
> This saves repeated re-parsing of format strings for every instance of a 
> formatted I/O operation, especially inside loops.
 >
> Regression tested on x86-64-linux-gnu.
> 
> OK for commit to fortran-dev branch?  If anyone is curious, the patch 
> will apply on 4.4 and 4.5 with some fuzz.

thanks for this nice patch!  Ok.

Some comments though:

+  hash &= (FORMAT_HASH_SIZE - 1);

I'd personally prefer

hash %= FORMAT_HASH_SIZE;

because this also works if someone sets FORMAT_HASH_SIZE to a 
non-power-of-two (even if that's improbable).  And if for some reason 
this occurrs, we're doing some nasty out-of-bounds accesses!

+  u->format_hash_table[hash].key = dtp->format;
+  u->format_hash_table[hash].key_len = dtp->format_len;

I'm no expert on these things, but it is ok to store dtp->format in the 
hash table by reference, I guess?  (Meaning that it is surely not freed 
before the hash-table entry is?)

@@ -156,7 +281,7 @@ free_format_data (st_parameter_dt *dtp)
      }

    free_mem (fmt);
-  dtp->u.p.fmt = NULL;
+  fmt = NULL;
  }

This last line is without effect now, you may want to remove it.

+/* revert()-- Do reversion of the format.  Control reverts to the left
+ * parenthesis that matches the rightmost right parenthesis.  From our
+ * tree structure, we are looking for the rightmost parenthesis node
+ * at the second level, the first level always being a single
+ * parenthesis node.  If this node doesn't exit, we use the top
+ * level. */

+  /* If r is NULL because no node was found, the whole tree will be used */

I think the right format for those is without each line starting with a 
star and to end with ".  ".

Yours,
Daniel

-- 
Done:  Arc-Bar-Cav-Ran-Rog-Sam-Tou-Val-Wiz
To go: Hea-Kni-Mon-Pri



More information about the Gcc-patches mailing list