Patchwork [pph] Independent pre-loaded cache for common nodes (issue4956041)

login
register
mail settings
Submitter Gab Charette
Date Aug. 25, 2011, 1:07 a.m.
Message ID <20110825010721.146FE1C13F5@gchare.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/111439/
State New
Headers show

Comments

Gab Charette - Aug. 25, 2011, 1:07 a.m.
This patch creates an independent cache, pph_preloaded_cache, which contains all common nodes which used to be directly added to each stream's cache.

The streams' cache now only contain nodes which really belong to them.

The preloaded cache is last in the lookup on the way out (potentially allowing us to play tricks later if we need to, i.e. by adding a modified preloaded structure's reference in the main cache to get the hit we want earlier and control the result...?)

Tested with bootstrap and pph regression testing.

Cheers,
Gab

2011-08-24  Gabriel Charette  <gchare@google.com>

	* pph-streamer-in.c (pph_is_reference_marker): Handle PPH_RECORD_PREF.
	(pph_in_start_record): Likewise.
	* pph-streamer-out.c (pph_out_start_record): Add extra lookup in
	pph_preloaded_cache if lookup in current and included streams failed.
	* pph-streamer.c (pph_preloaded_cache): New.
	(pph_cache_preload): Change first parameter to a pph_pickle_cache*
	from a pph_stream*. Update all users.
	(pph_cache_init): New.
	(pph_init_preloaded_cache): New local variable.
	(pph_stream_open): Use pph_cache_init. Do not call pph_cache_preload.
	(pph_cache_get): Add a new pph_record_marker parameter which we
	use to determine to type of get to do. Update all users.
	* pph-streamer.h (enum pph_record_marker): Add PPH_RECORD_PREF.
	(pph_in_record_marker): Handle PPH_RECORD_PREF.
	* pph.c (pph_init): Call pph_init_preloaded_cache.


--
This patch is available for review at http://codereview.appspot.com/4956041

Patch

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 720da6a..1561f88 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -145,12 +145,14 @@  pph_init_read (pph_stream *stream)
 }
 
 
-/* Return true if MARKER is either PPH_RECORD_IREF or PPH_RECORD_XREF.  */
+/* Return true if MARKER is PPH_RECORD_IREF, PPH_RECORD_XREF,
+   or PPH_RECORD_PREF.  */
 
 static inline bool
 pph_is_reference_marker (enum pph_record_marker marker)
 {
-  return marker == PPH_RECORD_IREF || marker == PPH_RECORD_XREF;
+  return marker == PPH_RECORD_IREF || marker == PPH_RECORD_XREF
+         || marker == PPH_RECORD_PREF;
 }
 
 
@@ -189,8 +191,10 @@  pph_in_start_record (pph_stream *stream, unsigned *include_ix_p,
 
   /* For PPH_RECORD_START and PPH_RECORD_IREF markers, read the
      streamer cache slot where we should store or find the
-     rematerialized data structure (see description above).  */
-  if (marker == PPH_RECORD_START || marker == PPH_RECORD_IREF)
+     rematerialized data structure (see description above).
+     Also read the preloaded cache slot in IX for PPH_RECORD_PREF.  */
+  if (marker == PPH_RECORD_START || marker == PPH_RECORD_IREF
+      || marker == PPH_RECORD_PREF)
     *cache_ix_p = pph_in_uint (stream);
   else if (marker == PPH_RECORD_XREF)
     {
@@ -407,13 +411,13 @@  pph_in_cxx_binding_1 (pph_stream *stream)
   cxx_binding *cb;
   tree value, type;
   enum pph_record_marker marker;
-  unsigned ix, include_ix;
+  unsigned ix, image_ix;
 
-  marker = pph_in_start_record (stream, &include_ix, &ix);
+  marker = pph_in_start_record (stream, &image_ix, &ix);
   if (marker == PPH_RECORD_END)
     return NULL;
   else if (pph_is_reference_marker (marker))
-    return (cxx_binding *) pph_cache_get (&stream->cache, include_ix, ix);
+    return (cxx_binding *) pph_cache_get (&stream->cache, image_ix, ix, marker);
 
   value = pph_in_tree (stream);
   type = pph_in_tree (stream);
@@ -461,7 +465,8 @@  pph_in_class_binding (pph_stream *stream)
   if (marker == PPH_RECORD_END)
     return NULL;
   else if (pph_is_reference_marker (marker))
-    return (cp_class_binding *) pph_cache_get (&stream->cache, image_ix, ix);
+    return (cp_class_binding *) pph_cache_get (&stream->cache, image_ix, ix,
+	                                       marker);
 
   ALLOC_AND_REGISTER (&stream->cache, ix, cb,
                       ggc_alloc_cleared_cp_class_binding ());
@@ -485,7 +490,8 @@  pph_in_label_binding (pph_stream *stream)
   if (marker == PPH_RECORD_END)
     return NULL;
   else if (pph_is_reference_marker (marker))
-    return (cp_label_binding *) pph_cache_get (&stream->cache, image_ix, ix);
+    return (cp_label_binding *) pph_cache_get (&stream->cache, image_ix, ix,
+	                                       marker);
 
   ALLOC_AND_REGISTER (&stream->cache, ix, lb,
                       ggc_alloc_cleared_cp_label_binding ());
@@ -526,7 +532,8 @@  pph_in_binding_level (pph_stream *stream, cp_binding_level *to_register)
   if (marker == PPH_RECORD_END)
     return NULL;
   else if (pph_is_reference_marker (marker))
-    return (cp_binding_level *) pph_cache_get (&stream->cache, image_ix, ix);
+    return (cp_binding_level *) pph_cache_get (&stream->cache, image_ix, ix,
+	                                       marker);
 
   /* If TO_REGISTER is set, register that binding level instead of the newly
      allocated binding level into slot IX.  */
@@ -604,7 +611,7 @@  pph_in_c_language_function (pph_stream *stream)
     return NULL;
   else if (pph_is_reference_marker (marker))
     return (struct c_language_function *) pph_cache_get (&stream->cache,
-							 image_ix, ix);
+							 image_ix, ix, marker);
 
   ALLOC_AND_REGISTER (&stream->cache, ix, clf,
 		      ggc_alloc_cleared_c_language_function ());
@@ -630,7 +637,7 @@  pph_in_language_function (pph_stream *stream)
     return NULL;
   else if (pph_is_reference_marker (marker))
     return (struct language_function *) pph_cache_get (&stream->cache,
-						       image_ix, ix);
+						       image_ix, ix, marker);
 
   ALLOC_AND_REGISTER (&stream->cache, ix, lf,
                       ggc_alloc_cleared_language_function ());
@@ -828,7 +835,8 @@  pph_in_lang_specific (pph_stream *stream, tree decl)
   else if (pph_is_reference_marker (marker))
     {
       DECL_LANG_SPECIFIC (decl) =
-	(struct lang_decl *) pph_cache_get (&stream->cache, image_ix, ix);
+	(struct lang_decl *) pph_cache_get (&stream->cache, image_ix, ix,
+	                                    marker);
       return;
     }
 
@@ -923,7 +931,7 @@  pph_in_sorted_fields_type (pph_stream *stream)
     return NULL;
   else if (pph_is_reference_marker (marker))
     return (struct sorted_fields_type *) pph_cache_get (&stream->cache,
-							image_ix, ix);
+							image_ix, ix, marker);
 
   num_fields = pph_in_uint (stream);
   ALLOC_AND_REGISTER (&stream->cache, ix, v,
@@ -1005,7 +1013,7 @@  pph_in_lang_type_class (pph_stream *stream, struct lang_type_class *ltc)
     }
   else if (pph_is_reference_marker (marker))
     ltc->nested_udts = (binding_table) pph_cache_get (&stream->cache,
-						      image_ix, ix);
+						      image_ix, ix, marker);
 
   ltc->as_base = pph_in_tree (stream);
   ltc->pure_virtuals = pph_in_tree_vec (stream);
@@ -1044,7 +1052,8 @@  pph_in_lang_type (pph_stream *stream)
   if (marker == PPH_RECORD_END)
     return NULL;
   else if (pph_is_reference_marker (marker))
-    return (struct lang_type *) pph_cache_get (&stream->cache, image_ix, ix);
+    return (struct lang_type *) pph_cache_get (&stream->cache, image_ix, ix,
+	                                       marker);
 
   ALLOC_AND_REGISTER (&stream->cache, ix, lt,
                       ggc_alloc_cleared_lang_type (sizeof (struct lang_type)));
@@ -1934,7 +1943,7 @@  pph_read_tree (struct lto_input_block *ib ATTRIBUTE_UNUSED,
   if (marker == PPH_RECORD_END)
     return NULL;
   else if (pph_is_reference_marker (marker))
-    return (tree) pph_cache_get (&stream->cache, image_ix, ix);
+    return (tree) pph_cache_get (&stream->cache, image_ix, ix, marker);
 
   /* We did not find the tree in the pickle cache, allocate the tree by
      reading the header fields (different tree nodes need to be
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index 83f98c3..5571cb0 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -219,6 +219,14 @@  pph_out_start_record (pph_stream *stream, void *data)
       return false;
     }
 
+  /* DATA is not in any stream's cache. See if it is a preloaded node.  */
+  if (pph_cache_lookup (NULL, data, &ix))
+    {
+      pph_out_record_marker (stream, PPH_RECORD_PREF);
+      pph_out_uint (stream, ix);
+      return false;
+    }
+
   /* DATA is in none of the pickle caches, add it to STREAM's pickle
      cache and tell the caller that it should pickle DATA out.  */
   pph_cache_add (&stream->cache, data, &ix);
diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
index 8ac3ddc..a29dac9 100644
--- a/gcc/cp/pph-streamer.c
+++ b/gcc/cp/pph-streamer.c
@@ -40,35 +40,32 @@  along with GCC; see the file COPYING3.  If not see
    them until the end of parsing (when pph_reader_finish is called).  */
 VEC(pph_stream_ptr, heap) *pph_read_images;
 
-/* Pre-load common tree nodes into the pickle cache in STREAM.  These
-   nodes are always built by the front end, so there is no need to
-   pickle them.
+/* A cache of pre-loaded common tree nodes.  */
+static pph_pickle_cache *pph_preloaded_cache;
 
-   FIXME pph - Every stream will have its own pickle cache.  Many
-   entries in all those caches will be the same.  Implement a common
-   cache for everything we preload here so that we do not have to
-   preload every cache we instantiate.  */
+/* Pre-load common tree nodes into CACHE.  These nodes are always built by the
+   front end, so there is no need to pickle them.  */
 
 static void
-pph_cache_preload (pph_stream *stream)
+pph_cache_preload (pph_pickle_cache *cache)
 {
   unsigned i;
 
   for (i = itk_char; i < itk_none; i++)
-    pph_cache_add (&stream->cache, integer_types[i], NULL);
+    pph_cache_add (cache, integer_types[i], NULL);
 
   for (i = 0; i < TYPE_KIND_LAST; i++)
-    pph_cache_add (&stream->cache, sizetype_tab[i], NULL);
+    pph_cache_add (cache, sizetype_tab[i], NULL);
 
   /* global_trees[] can have NULL entries in it.  Skip them.  */
   for (i = 0; i < TI_MAX; i++)
     if (global_trees[i])
-      pph_cache_add (&stream->cache, global_trees[i], NULL);
+      pph_cache_add (cache, global_trees[i], NULL);
 
   /* c_global_trees[] can have NULL entries in it.  Skip them.  */
   for (i = 0; i < CTI_MAX; i++)
     if (c_global_trees[i])
-      pph_cache_add (&stream->cache, c_global_trees[i], NULL);
+      pph_cache_add (cache, c_global_trees[i], NULL);
 
   /* cp_global_trees[] can have NULL entries in it.  Skip them.  */
   for (i = 0; i < CPTI_MAX; i++)
@@ -78,13 +75,13 @@  pph_cache_preload (pph_stream *stream)
 	continue;
 
       if (cp_global_trees[i])
-	pph_cache_add (&stream->cache, cp_global_trees[i], NULL);
+	pph_cache_add (cache, cp_global_trees[i], NULL);
     }
 
   /* Add other well-known nodes that should always be taken from the
      current compilation context.  */
-  pph_cache_add (&stream->cache, global_namespace, NULL);
-  pph_cache_add (&stream->cache, DECL_CONTEXT (global_namespace), NULL);
+  pph_cache_add (cache, global_namespace, NULL);
+  pph_cache_add (cache, DECL_CONTEXT (global_namespace), NULL);
 }
 
 
@@ -101,6 +98,25 @@  pph_hooks_init (void)
 }
 
 
+/* Initialize an empty pickle CACHE.  */
+
+static void
+pph_cache_init (pph_pickle_cache *cache)
+{
+  cache->v = NULL;
+  cache->m = pointer_map_create ();
+}
+
+
+void
+pph_init_preloaded_cache (void)
+{
+  pph_preloaded_cache = XCNEW (pph_pickle_cache);
+  pph_cache_init (pph_preloaded_cache);
+  pph_cache_preload (pph_preloaded_cache);
+}
+
+
 /* Create a new PPH stream to be stored on the file called NAME.
    MODE is passed to fopen directly.  */
 
@@ -120,15 +136,13 @@  pph_stream_open (const char *name, const char *mode)
   stream->file = f;
   stream->name = xstrdup (name);
   stream->write_p = (strchr (mode, 'w') != NULL);
-  stream->cache.m = pointer_map_create ();
+  pph_cache_init (&stream->cache);
   stream->symtab.m = pointer_set_create ();
   if (stream->write_p)
     pph_init_write (stream);
   else
     pph_init_read (stream);
 
-  pph_cache_preload (stream);
-
   return stream;
 }
 
@@ -400,7 +414,8 @@  pph_cache_insert_at (pph_pickle_cache *cache, void *data, unsigned ix)
 
 
 /* Return true if DATA exists in CACHE.  If IX_P is not NULL, store the cache
-   slot where DATA resides in *IX_P (or (unsigned)-1 if DATA is not found).  */
+   slot where DATA resides in *IX_P (or (unsigned)-1 if DATA is not found).
+   If CACHE is NULL use pph_preloaded_cache by default.  */
 
 bool
 pph_cache_lookup (pph_pickle_cache *cache, void *data, unsigned *ix_p)
@@ -409,6 +424,9 @@  pph_cache_lookup (pph_pickle_cache *cache, void *data, unsigned *ix_p)
   unsigned ix;
   bool existed_p;
 
+  if (cache == NULL)
+    cache = pph_preloaded_cache;
+
   map_slot = pointer_map_contains (cache->m, data);
   if (map_slot == NULL)
     {
@@ -502,24 +520,35 @@  pph_cache_add (pph_pickle_cache *cache, void *data, unsigned *ix_p)
 }
 
 
-/* Return the pointer at slot IX in STREAM's pickle cache.  If INCLUDE_IX
-   is not -1U, then instead of looking up in STREAM's pickle cache,
-   instead of looking up in CACHE, the pointer is looked up in the CACHE of
-   pph_read_images[INCLUDE_IX].  */
+/* Return the pointer at slot IX in STREAM_CACHE if MARKER is an IREF.  IF
+   MARKER is a XREF then instead of looking up in STREAM_CACHE, the pointer is
+   looked up in the CACHE of pph_read_images[INCLUDE_IX].  Finally if MARKER is
+   a PREF return the pointer at slot IX in the preloaded cache.  */
 
 void *
-pph_cache_get (pph_pickle_cache *cache, unsigned include_ix, unsigned ix)
+pph_cache_get (pph_pickle_cache *stream_cache, unsigned include_ix, unsigned ix,
+               enum pph_record_marker marker)
 {
   void *data;
-  pph_pickle_cache *img_cache;
+  pph_pickle_cache *cache;
 
-  /* Determine which image's pickle cache to use.  */
-  if (include_ix == (unsigned) -1)
-    img_cache = cache;
-  else
-    img_cache = &VEC_index (pph_stream_ptr, pph_read_images, include_ix)->cache;
+  /* Determine which cache to use.  */
+  switch (marker)
+    {
+    case PPH_RECORD_IREF:
+      cache = stream_cache;
+      break;
+    case PPH_RECORD_XREF:
+      cache = &VEC_index (pph_stream_ptr, pph_read_images, include_ix)->cache;
+      break;
+    case PPH_RECORD_PREF:
+      cache = pph_preloaded_cache;
+      break;
+    default:
+      gcc_unreachable ();
+    }
 
-  data = VEC_index (void_p, img_cache->v, ix);
+  data = VEC_index (void_p, cache->v, ix);
   gcc_assert (data);
   return data;
 }
diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
index a51db3b..6ee643e 100644
--- a/gcc/cp/pph-streamer.h
+++ b/gcc/cp/pph-streamer.h
@@ -47,7 +47,12 @@  enum pph_record_marker {
      indices: (1) the index into the include table where the other
      image lives, and (2) the cache slot into that image's pickle
      cache where the data resides.  */
-  PPH_RECORD_XREF
+  PPH_RECORD_XREF,
+
+  /* Preloaded reference. This marker indicates that this data is a preloaded
+     node created by the front-end at the beginning of compilation, which we
+     do not need to stream out as it will already exist on the way in.  */
+  PPH_RECORD_PREF
 };
 
 /* Symbol table markers.  These are all the symbols that need to be 
@@ -213,6 +218,7 @@  typedef struct pph_stream {
 #define PPHF_NO_XREFS		(1 << 1)
 
 /* In pph-streamer.c.  */
+void pph_init_preloaded_cache (void);
 pph_stream *pph_stream_open (const char *, const char *);
 void pph_stream_close (pph_stream *);
 void pph_trace_tree (pph_stream *, tree);
@@ -227,7 +233,8 @@  void pph_cache_insert_at (pph_pickle_cache *, void *, unsigned);
 bool pph_cache_lookup (pph_pickle_cache *, void *, unsigned *);
 bool pph_cache_lookup_in_includes (void *, unsigned *, unsigned *);
 bool pph_cache_add (pph_pickle_cache *, void *, unsigned *);
-void *pph_cache_get (pph_pickle_cache *, unsigned, unsigned);
+void *pph_cache_get (pph_pickle_cache *, unsigned, unsigned,
+                     enum pph_record_marker);
 
 /* In pph-streamer-out.c.  */
 void pph_flush_buffers (pph_stream *);
@@ -529,7 +536,8 @@  pph_in_record_marker (pph_stream *stream)
   gcc_assert (m == PPH_RECORD_START
 	      || m == PPH_RECORD_END
 	      || m == PPH_RECORD_IREF
-	      || m == PPH_RECORD_XREF);
+	      || m == PPH_RECORD_XREF
+	      || m == PPH_RECORD_PREF);
   return m;
 }
 
diff --git a/gcc/cp/pph.c b/gcc/cp/pph.c
index 74f48fc..9b1c63c 100644
--- a/gcc/cp/pph.c
+++ b/gcc/cp/pph.c
@@ -168,6 +168,8 @@  pph_init (void)
   if (pph_out_file != NULL)
     pph_writer_init ();
 
+  pph_init_preloaded_cache ();
+
   pph_read_images = NULL;
 }