Re: [PATCH 4/5] add finalizers to ggc

[Trevor Saunders <tsaunders at mozilla dot com>]

On Tue, May 13, 2014 at 11:41:46AM +0200, Richard Biener wrote:
> On Tue, May 13, 2014 at 8:41 AM,  <tsaunders@mozilla.com> wrote:
> > From: Trevor Saunders <tsaunders@mozilla.com>
> >
> > This implements finalizers by keeping a list of registered finalizers
> > and after every mark but before sweeping check to see if any of them are
> > for unmarked blocks.
> >
> > This uses the two vector and forward iteration approach I think richi agreed to.
> >
> > bootstrapped + regtested on x86_64-unknown-linux-gnu ok?
> 
> Ok with a comment before ggc_handle_finalizers.

patches 1-4/5 committed as r2105655 - r210568 and comment for
ggc_handle_finalizers committed as r210569

thanks!

Trev
	
> 
> Thanks,
> Richard.
> 
> > Trev
> >
> > gcc/ChangeLog:
> >
> >         * ggc-common.c (ggc_internal_cleared_alloc): Adjust.
> >         * ggc-none.c (ggc_internal_alloc): Assert if a finalizer is passed.
> >         (ggc_internal_cleared_alloc): Likewise.
> >         * ggc-page.c (finalizer): New class.
> > (vec_finalizer): Likewise.
> >         (globals::finalizers): New member.
> > (globals::vec_finalizers): Likewise.
> >         (ggc_internal_alloc): Record the finalizer if any for the block being
> >         allocated.
> >         (ggc_handle_finalizers): New function.
> >         (ggc_collect): Call ggc_handle_finalizers.
> >         * ggc.h (ggc_internal_alloc): Add arguments to allow installing a
> >         finalizer.
> >         (ggc_internal_cleared_alloc): Likewise.
> >         (finalize): New function.
> >         (need_finalization_p): Likewise.
> >         (ggc_alloc): Install the type's destructor as the finalizer if it
> >         might do something.
> >         (ggc_cleared_alloc): Likewise.
> >         (ggc_vec_alloc): Likewise.
> >         (ggc_cleared_vec_alloc): Likewise.
> > ---
> >  gcc/ggc-common.c |  5 ++--
> >  gcc/ggc-none.c   |  8 ++++--
> >  gcc/ggc-page.c   | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  gcc/ggc.h        | 71 +++++++++++++++++++++++++++++++++++++++------
> >  4 files changed, 158 insertions(+), 13 deletions(-)
> >
> > diff --git a/gcc/ggc-common.c b/gcc/ggc-common.c
> > index e89cc64..b11a10c 100644
> > --- a/gcc/ggc-common.c
> > +++ b/gcc/ggc-common.c
> > @@ -174,9 +174,10 @@ ggc_mark_roots (void)
> >
> >  /* Allocate a block of memory, then clear it.  */
> >  void *
> > -ggc_internal_cleared_alloc (size_t size MEM_STAT_DECL)
> > +ggc_internal_cleared_alloc (size_t size, void (*f)(void *), size_t s, size_t n
> > +                           MEM_STAT_DECL)
> >  {
> > -  void *buf = ggc_internal_alloc (size PASS_MEM_STAT);
> > +  void *buf = ggc_internal_alloc (size, f, s, n PASS_MEM_STAT);
> >    memset (buf, 0, size);
> >    return buf;
> >  }
> > diff --git a/gcc/ggc-none.c b/gcc/ggc-none.c
> > index aad89bf..97d3566 100644
> > --- a/gcc/ggc-none.c
> > +++ b/gcc/ggc-none.c
> > @@ -41,14 +41,18 @@ ggc_round_alloc_size (size_t requested_size)
> >  }
> >
> >  void *
> > -ggc_internal_alloc (size_t size MEM_STAT_DECL)
> > +ggc_internal_alloc (size_t size, void (*f)(void *), size_t, size_t
> > +                   MEM_STAT_DECL)
> >  {
> > +  gcc_assert (!f); // ggc-none doesn't support finalizers
> >    return xmalloc (size);
> >  }
> >
> >  void *
> > -ggc_internal_cleared_alloc (size_t size MEM_STAT_DECL)
> > +ggc_internal_cleared_alloc (size_t size, void (*f)(void *), size_t, size_t
> > +                           MEM_STAT_DECL)
> >  {
> > +  gcc_assert (!f); // ggc-none doesn't support finalizers
> >    return xcalloc (size, 1);
> >  }
> >
> > diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c
> > index ae5e88a..b3a1a2a 100644
> > --- a/gcc/ggc-page.c
> > +++ b/gcc/ggc-page.c
> > @@ -332,6 +332,41 @@ typedef struct page_table_chain
> >
> >  #endif
> >
> > +class finalizer
> > +{
> > +public:
> > +  finalizer (void *addr, void (*f)(void *)) : m_addr (addr), m_function (f) {}
> > +
> > +  void *addr () const { return m_addr; }
> > +
> > +  void call () const { m_function (m_addr); }
> > +
> > +private:
> > +  void *m_addr;
> > +  void (*m_function)(void *);
> > +};
> > +
> > +class vec_finalizer
> > +{
> > +public:
> > +  vec_finalizer (uintptr_t addr, void (*f)(void *), size_t s, size_t n) :
> > +    m_addr (addr), m_function (f), m_object_size (s), m_n_objects (n) {}
> > +
> > +  void call () const
> > +    {
> > +      for (size_t i = 0; i < m_n_objects; i++)
> > +       m_function (reinterpret_cast<void *> (m_addr + (i * m_object_size)));
> > +    }
> > +
> > +  void *addr () const { return reinterpret_cast<void *> (m_addr); }
> > +
> > +private:
> > +  uintptr_t m_addr;
> > +  void (*m_function)(void *);
> > +  size_t m_object_size;
> > +  size_t m_n_objects;
> > +  };
> > +
> >  #ifdef ENABLE_GC_ALWAYS_COLLECT
> >  /* List of free objects to be verified as actually free on the
> >     next collection.  */
> > @@ -425,6 +460,12 @@ static struct globals
> >       better runtime data access pattern.  */
> >    unsigned long **save_in_use;
> >
> > +  /* Finalizers for single objects.  */
> > +  vec<finalizer> finalizers;
> > +
> > +  /* Finalizers for vectors of objects.  */
> > +  vec<vec_finalizer> vec_finalizers;
> > +
> >  #ifdef ENABLE_GC_ALWAYS_COLLECT
> >    /* List of free objects to be verified as actually free on the
> >       next collection.  */
> > @@ -1202,7 +1243,8 @@ ggc_round_alloc_size (size_t requested_size)
> >  /* Allocate a chunk of memory of SIZE bytes.  Its contents are undefined.  */
> >
> >  void *
> > -ggc_internal_alloc (size_t size MEM_STAT_DECL)
> > +ggc_internal_alloc (size_t size, void (*f)(void *), size_t s, size_t n
> > +                   MEM_STAT_DECL)
> >  {
> >    size_t order, word, bit, object_offset, object_size;
> >    struct page_entry *entry;
> > @@ -1345,6 +1387,12 @@ ggc_internal_alloc (size_t size MEM_STAT_DECL)
> >    /* For timevar statistics.  */
> >    timevar_ggc_mem_total += object_size;
> >
> > +  if (f && n == 1)
> > +    G.finalizers.safe_push (finalizer (result, f));
> > +  else if (f)
> > +    G.vec_finalizers.safe_push
> > +      (vec_finalizer (reinterpret_cast<uintptr_t> (result), f, s, n));
> > +
> >    if (GATHER_STATISTICS)
> >      {
> >        size_t overhead = object_size - size;
> > @@ -1811,6 +1859,42 @@ clear_marks (void)
> >      }
> >  }
> >
> > +static void
> > +ggc_handle_finalizers ()
> > +{
> > +  if (G.context_depth != 0)
> > +    return;
> > +
> > +  unsigned length = G.finalizers.length ();
> > +  for (unsigned int i = 0; i < length;)
> > +    {
> > +      finalizer &f = G.finalizers[i];
> > +      if (!ggc_marked_p (f.addr ()))
> > +       {
> > +         f.call ();
> > +         G.finalizers.unordered_remove (i);
> > +         length--;
> > +       }
> > +      else
> > +       i++;
> > +    }
> > +
> > +
> > +  length = G.vec_finalizers.length ();
> > +  for (unsigned int i = 0; i < length;)
> > +    {
> > +      vec_finalizer &f = G.vec_finalizers[i];
> > +      if (!ggc_marked_p (f.addr ()))
> > +       {
> > +         f.call ();
> > +         G.vec_finalizers.unordered_remove (i);
> > +         length--;
> > +       }
> > +      else
> > +       i++;
> > +    }
> > +}
> > +
> >  /* Free all empty pages.  Partially empty pages need no attention
> >     because the `mark' bit doubles as an `unused' bit.  */
> >
> > @@ -2075,6 +2159,7 @@ ggc_collect (void)
> >
> >    clear_marks ();
> >    ggc_mark_roots ();
> > +  ggc_handle_finalizers ();
> >
> >    if (GATHER_STATISTICS)
> >      ggc_prune_overhead_list ();
> > diff --git a/gcc/ggc.h b/gcc/ggc.h
> > index 50fb199..1279aee 100644
> > --- a/gcc/ggc.h
> > +++ b/gcc/ggc.h
> > @@ -136,13 +136,30 @@ extern void gt_pch_save (FILE *f);
> >  /* Allocation.  */
> >
> >  /* The internal primitive.  */
> > -extern void *ggc_internal_alloc (size_t CXX_MEM_STAT_INFO) ATTRIBUTE_MALLOC;
> > +extern void *ggc_internal_alloc (size_t, void (*)(void *), size_t,
> > +                                size_t CXX_MEM_STAT_INFO)
> > +     ATTRIBUTE_MALLOC;
> > +
> > +     static inline
> > +     void *
> > +     ggc_internal_alloc (size_t s CXX_MEM_STAT_INFO)
> > +{
> > +  return ggc_internal_alloc (s, NULL, 0, 1 PASS_MEM_STAT);
> > +}
> >
> >  extern size_t ggc_round_alloc_size (size_t requested_size);
> >
> >  /* Allocates cleared memory.  */
> > -extern void *ggc_internal_cleared_alloc (size_t CXX_MEM_STAT_INFO)
> > -  ATTRIBUTE_MALLOC;
> > +extern void *ggc_internal_cleared_alloc (size_t, void (*)(void *),
> > +                                        size_t, size_t
> > +                                        CXX_MEM_STAT_INFO) ATTRIBUTE_MALLOC;
> > +
> > +static inline
> > +void *
> > +ggc_internal_cleared_alloc (size_t s CXX_MEM_STAT_INFO)
> > +{
> > +  return ggc_internal_cleared_alloc (s, NULL, 0, 1 PASS_MEM_STAT);
> > +}
> >
> >  /* Resize a block.  */
> >  extern void *ggc_realloc (void *, size_t CXX_MEM_STAT_INFO);
> > @@ -157,25 +174,57 @@ extern void dump_ggc_loc_statistics (bool);
> >      ((T *) ggc_realloc ((P), (N) * sizeof (T) MEM_STAT_INFO))
> >
> >  template<typename T>
> > +void
> > +finalize (void *p)
> > +{
> > +  static_cast<T *> (p)->~T ();
> > +}
> > +
> > +template<typename T>
> > +static inline bool
> > +need_finalization_p ()
> > +{
> > +#if GCC_VERSION >= 4003
> > +  return !__has_trivial_destructor (T);
> > +#else
> > +  return true;
> > +#endif
> > +}
> > +
> > +template<typename T>
> >  static inline T *
> >  ggc_alloc (ALONE_CXX_MEM_STAT_INFO)
> >  {
> > -  return static_cast<T *> (ggc_internal_alloc (sizeof (T) PASS_MEM_STAT));
> > +  if (need_finalization_p<T> ())
> > +    return static_cast<T *> (ggc_internal_alloc (sizeof (T), finalize<T>, 0, 1
> > +                                                PASS_MEM_STAT));
> > +  else
> > +    return static_cast<T *> (ggc_internal_alloc (sizeof (T), NULL, 0, 1
> > +                                                PASS_MEM_STAT));
> >  }
> >
> >  template<typename T>
> >  static inline T *
> >  ggc_cleared_alloc (ALONE_CXX_MEM_STAT_INFO)
> >  {
> > -  return static_cast<T *> (ggc_internal_cleared_alloc (sizeof (T)
> > -                                                      PASS_MEM_STAT));
> > +  if (need_finalization_p<T> ())
> > +    return static_cast<T *> (ggc_internal_cleared_alloc (sizeof (T),
> > +                                                        finalize<T>, 0, 1
> > +                                                        PASS_MEM_STAT));
> > +  else
> > +    return static_cast<T *> (ggc_internal_cleared_alloc (sizeof (T), NULL, 0, 1
> > +                                                        PASS_MEM_STAT));
> >  }
> >
> >  template<typename T>
> >  static inline T *
> >  ggc_vec_alloc (size_t c CXX_MEM_STAT_INFO)
> >  {
> > -    return static_cast<T *> (ggc_internal_alloc (c * sizeof (T)
> > +  if (need_finalization_p<T> ())
> > +    return static_cast<T *> (ggc_internal_alloc (c * sizeof (T), finalize<T>,
> > +                                                sizeof (T), c PASS_MEM_STAT));
> > +  else
> > +    return static_cast<T *> (ggc_internal_alloc (c * sizeof (T), NULL, 0, 0
> >                                                  PASS_MEM_STAT));
> >  }
> >
> > @@ -183,8 +232,14 @@ template<typename T>
> >  static inline T *
> >  ggc_cleared_vec_alloc (size_t c CXX_MEM_STAT_INFO)
> >  {
> > -    return static_cast<T *> (ggc_internal_cleared_alloc (c * sizeof (T)
> > +  if (need_finalization_p<T> ())
> > +    return static_cast<T *> (ggc_internal_cleared_alloc (c * sizeof (T),
> > +                                                        finalize<T>,
> > +                                                        sizeof (T), c
> >                                                          PASS_MEM_STAT));
> > +  else
> > +    return static_cast<T *> (ggc_internal_cleared_alloc (c * sizeof (T), NULL,
> > +                                                        0, 0 PASS_MEM_STAT));
> >  }
> >
> >  static inline void *
> > --
> > 2.0.0.rc2
> >

Attachment: signature.asc
Description: Digital signature