Patchwork libgcov support for profile collection in region of interest (issue6186044)

login
register
mail settings
Submitter Teresa Johnson
Date May 3, 2012, 5:52 p.m.
Message ID <20120503175253.8452160BD3@tjsboxrox.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/156750/
State New
Headers show

Comments

Teresa Johnson - May 3, 2012, 5:52 p.m.
This patch adds functionality to libgcov to enable user applications to
collect profile data only in regions of interest. This is useful, for
example, to collect profile data from a long-running server only
during the time when it is serving requests.

Specifically, the new routines __gcov_reset will clear all profile counters
to zero and __gcov_dump will write out the profile information collected so
far. A global variable is used to prevent writing out the profile a
second time during exit.

Bootstrapped and tested on x86_64-unknown-linux-gnu.  Is this ok for trunk?

Thanks,
Teresa

2012-05-03   Teresa Johnson  <tejohnson@google.com>

	* libgcc/libgcov.c (gcov_clear, __gcov_reset): New functions.
	(__gcov_dump): Ditto.
	(gcov_dump_complete): New global variable.
	(__gcov_flush): Outline functionality now in gcov_clear.
	* gcc/gcov-io.h (__gcov_reset, __gcov_dump): Declare.


--
This patch is available for review at http://codereview.appspot.com/6186044
Xinliang David Li - May 7, 2012, 4:06 p.m.
+Honza and Nathan.

David

On Thu, May 3, 2012 at 10:52 AM, Teresa Johnson <tejohnson@google.com> wrote:
> This patch adds functionality to libgcov to enable user applications to
> collect profile data only in regions of interest. This is useful, for
> example, to collect profile data from a long-running server only
> during the time when it is serving requests.
>
> Specifically, the new routines __gcov_reset will clear all profile counters
> to zero and __gcov_dump will write out the profile information collected so
> far. A global variable is used to prevent writing out the profile a
> second time during exit.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.  Is this ok for trunk?
>
> Thanks,
> Teresa
>
> 2012-05-03   Teresa Johnson  <tejohnson@google.com>
>
>        * libgcc/libgcov.c (gcov_clear, __gcov_reset): New functions.
>        (__gcov_dump): Ditto.
>        (gcov_dump_complete): New global variable.
>        (__gcov_flush): Outline functionality now in gcov_clear.
>        * gcc/gcov-io.h (__gcov_reset, __gcov_dump): Declare.
>
> Index: libgcc/libgcov.c
> ===================================================================
> --- libgcc/libgcov.c    (revision 187048)
> +++ libgcc/libgcov.c    (working copy)
> @@ -48,6 +48,8 @@ see the files COPYING3 and COPYING.RUNTIME respect
>  #ifdef L_gcov
>  void __gcov_init (struct gcov_info *p __attribute__ ((unused))) {}
>  void __gcov_flush (void) {}
> +void __gcov_reset (void) {}
> +void __gcov_dump (void) {}
>  #endif
>
>  #ifdef L_gcov_merge_add
> @@ -91,6 +93,9 @@ static struct gcov_info *gcov_list;
>  /* Size of the longest file name. */
>  static size_t gcov_max_filename = 0;
>
> +/* Flag when the profile has already been dumped via __gcov_dump().  */
> +static int gcov_dump_complete = 0;
> +
>  /* Make sure path component of the given FILENAME exists, create
>    missing directories. FILENAME must be writable.
>    Returns zero on success, or -1 if an error occurred.  */
> @@ -286,6 +291,11 @@ gcov_exit (void)
>   char *gi_filename, *gi_filename_up;
>   gcov_unsigned_t crc32 = 0;
>
> +  /* Prevent the counters from being dumped a second time on exit when the
> +     application already wrote out the profile using __gcov_dump().  */
> +  if (gcov_dump_complete)
> +    return;
> +
>   memset (&all_prg, 0, sizeof (all_prg));
>   /* Find the totals for this execution.  */
>   memset (&this_prg, 0, sizeof (this_prg));
> @@ -679,6 +689,37 @@ gcov_exit (void)
>     }
>  }
>
> +/* Reset all counters to zero.  */
> +
> +static void
> +gcov_clear (void)
> +{
> +  const struct gcov_info *gi_ptr;
> +
> +  for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
> +    {
> +      unsigned f_ix;
> +
> +      for (f_ix = 0; f_ix < gi_ptr->n_functions; f_ix++)
> +       {
> +         unsigned t_ix;
> +         const struct gcov_fn_info *gfi_ptr = gi_ptr->functions[f_ix];
> +
> +         if (!gfi_ptr || gfi_ptr->key != gi_ptr)
> +           continue;
> +         const struct gcov_ctr_info *ci_ptr = gfi_ptr->ctrs;
> +         for (t_ix = 0; t_ix != GCOV_COUNTERS; t_ix++)
> +           {
> +             if (!gi_ptr->merge[t_ix])
> +               continue;
> +
> +             memset (ci_ptr->values, 0, sizeof (gcov_type) * ci_ptr->num);
> +             ci_ptr++;
> +           }
> +       }
> +    }
> +}
> +
>  /* Add a new object file onto the bb chain.  Invoked automatically
>    when running an object file's global ctors.  */
>
> @@ -730,38 +771,38 @@ init_mx_once (void)
>  void
>  __gcov_flush (void)
>  {
> -  const struct gcov_info *gi_ptr;
> -
>   init_mx_once ();
>   __gthread_mutex_lock (&__gcov_flush_mx);
>
>   gcov_exit ();
> -  for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
> -    {
> -      unsigned f_ix;
> +  gcov_clear ();
>
> -      for (f_ix = 0; f_ix < gi_ptr->n_functions; f_ix++)
> -       {
> -         unsigned t_ix;
> -         const struct gcov_fn_info *gfi_ptr = gi_ptr->functions[f_ix];
> +  __gthread_mutex_unlock (&__gcov_flush_mx);
> +}
>
> -         if (!gfi_ptr || gfi_ptr->key != gi_ptr)
> -           continue;
> -         const struct gcov_ctr_info *ci_ptr = gfi_ptr->ctrs;
> -         for (t_ix = 0; t_ix != GCOV_COUNTERS; t_ix++)
> -           {
> -             if (!gi_ptr->merge[t_ix])
> -               continue;
> -
> -             memset (ci_ptr->values, 0, sizeof (gcov_type) * ci_ptr->num);
> -             ci_ptr++;
> -           }
> -       }
> -    }
> +/* Function that can be called from application to reset counters to zero,
> +   in order to collect profile in region of interest.  */
>
> -  __gthread_mutex_unlock (&__gcov_flush_mx);
> +void
> +__gcov_reset (void)
> +{
> +  gcov_clear ();
> +  /* Re-enable dumping to support collecting profile in multiple regions
> +     of interest.  */
> +  gcov_dump_complete = 0;
>  }
>
> +/* Function that can be called from application to write profile collected
> +   so far, in order to collect profile in region of interest.  */
> +
> +void
> +__gcov_dump (void)
> +{
> +  gcov_exit ();
> +  /* Prevent profile from being dumped a second time on application exit.  */
> +  gcov_dump_complete = 1;
> +}
> +
>  #endif /* L_gcov */
>
>  #ifdef L_gcov_merge_add
> Index: gcc/gcov-io.h
> ===================================================================
> --- gcc/gcov-io.h       (revision 187048)
> +++ gcc/gcov-io.h       (working copy)
> @@ -458,6 +458,12 @@ extern void __gcov_init (struct gcov_info *) ATTRI
>  /* Called before fork, to avoid double counting.  */
>  extern void __gcov_flush (void) ATTRIBUTE_HIDDEN;
>
> +/* Function to reset all counters to 0.  */
> +extern void __gcov_reset (void);
> +
> +/* Function to enable early write of profile information so far.  */
> +extern void __gcov_dump (void);
> +
>  /* The merge function that just sums the counters.  */
>  extern void __gcov_merge_add (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
>
>
> --
> This patch is available for review at http://codereview.appspot.com/6186044
Jan Hubicka - May 7, 2012, 11:36 p.m.
> +Honza and Nathan.
> 
> David
> 
> On Thu, May 3, 2012 at 10:52 AM, Teresa Johnson <tejohnson@google.com> wrote:
> > This patch adds functionality to libgcov to enable user applications to
> > collect profile data only in regions of interest. This is useful, for
> > example, to collect profile data from a long-running server only
> > during the time when it is serving requests.
> >
> > Specifically, the new routines __gcov_reset will clear all profile counters
> > to zero and __gcov_dump will write out the profile information collected so
> > far. A global variable is used to prevent writing out the profile a
> > second time during exit.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.  Is this ok for trunk?

This seems resonable things to do.  You need to add an documentation and make sure they
go new L_* section, since not every app using libgcov needs necesarily those two.

Honza
> >
> > Thanks,
> > Teresa
> >
> > 2012-05-03   Teresa Johnson  <tejohnson@google.com>
> >
> >        * libgcc/libgcov.c (gcov_clear, __gcov_reset): New functions.
> >        (__gcov_dump): Ditto.
> >        (gcov_dump_complete): New global variable.
> >        (__gcov_flush): Outline functionality now in gcov_clear.
> >        * gcc/gcov-io.h (__gcov_reset, __gcov_dump): Declare.
> >
> > Index: libgcc/libgcov.c
> > ===================================================================
> > --- libgcc/libgcov.c    (revision 187048)
> > +++ libgcc/libgcov.c    (working copy)
> > @@ -48,6 +48,8 @@ see the files COPYING3 and COPYING.RUNTIME respect
> >  #ifdef L_gcov
> >  void __gcov_init (struct gcov_info *p __attribute__ ((unused))) {}
> >  void __gcov_flush (void) {}
> > +void __gcov_reset (void) {}
> > +void __gcov_dump (void) {}
> >  #endif
> >
> >  #ifdef L_gcov_merge_add
> > @@ -91,6 +93,9 @@ static struct gcov_info *gcov_list;
> >  /* Size of the longest file name. */
> >  static size_t gcov_max_filename = 0;
> >
> > +/* Flag when the profile has already been dumped via __gcov_dump().  */
> > +static int gcov_dump_complete = 0;
> > +
> >  /* Make sure path component of the given FILENAME exists, create
> >    missing directories. FILENAME must be writable.
> >    Returns zero on success, or -1 if an error occurred.  */
> > @@ -286,6 +291,11 @@ gcov_exit (void)
> >   char *gi_filename, *gi_filename_up;
> >   gcov_unsigned_t crc32 = 0;
> >
> > +  /* Prevent the counters from being dumped a second time on exit when the
> > +     application already wrote out the profile using __gcov_dump().  */
> > +  if (gcov_dump_complete)
> > +    return;
> > +
> >   memset (&all_prg, 0, sizeof (all_prg));
> >   /* Find the totals for this execution.  */
> >   memset (&this_prg, 0, sizeof (this_prg));
> > @@ -679,6 +689,37 @@ gcov_exit (void)
> >     }
> >  }
> >
> > +/* Reset all counters to zero.  */
> > +
> > +static void
> > +gcov_clear (void)
> > +{
> > +  const struct gcov_info *gi_ptr;
> > +
> > +  for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
> > +    {
> > +      unsigned f_ix;
> > +
> > +      for (f_ix = 0; f_ix < gi_ptr->n_functions; f_ix++)
> > +       {
> > +         unsigned t_ix;
> > +         const struct gcov_fn_info *gfi_ptr = gi_ptr->functions[f_ix];
> > +
> > +         if (!gfi_ptr || gfi_ptr->key != gi_ptr)
> > +           continue;
> > +         const struct gcov_ctr_info *ci_ptr = gfi_ptr->ctrs;
> > +         for (t_ix = 0; t_ix != GCOV_COUNTERS; t_ix++)
> > +           {
> > +             if (!gi_ptr->merge[t_ix])
> > +               continue;
> > +
> > +             memset (ci_ptr->values, 0, sizeof (gcov_type) * ci_ptr->num);
> > +             ci_ptr++;
> > +           }
> > +       }
> > +    }
> > +}
> > +
> >  /* Add a new object file onto the bb chain.  Invoked automatically
> >    when running an object file's global ctors.  */
> >
> > @@ -730,38 +771,38 @@ init_mx_once (void)
> >  void
> >  __gcov_flush (void)
> >  {
> > -  const struct gcov_info *gi_ptr;
> > -
> >   init_mx_once ();
> >   __gthread_mutex_lock (&__gcov_flush_mx);
> >
> >   gcov_exit ();
> > -  for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
> > -    {
> > -      unsigned f_ix;
> > +  gcov_clear ();
> >
> > -      for (f_ix = 0; f_ix < gi_ptr->n_functions; f_ix++)
> > -       {
> > -         unsigned t_ix;
> > -         const struct gcov_fn_info *gfi_ptr = gi_ptr->functions[f_ix];
> > +  __gthread_mutex_unlock (&__gcov_flush_mx);
> > +}
> >
> > -         if (!gfi_ptr || gfi_ptr->key != gi_ptr)
> > -           continue;
> > -         const struct gcov_ctr_info *ci_ptr = gfi_ptr->ctrs;
> > -         for (t_ix = 0; t_ix != GCOV_COUNTERS; t_ix++)
> > -           {
> > -             if (!gi_ptr->merge[t_ix])
> > -               continue;
> > -
> > -             memset (ci_ptr->values, 0, sizeof (gcov_type) * ci_ptr->num);
> > -             ci_ptr++;
> > -           }
> > -       }
> > -    }
> > +/* Function that can be called from application to reset counters to zero,
> > +   in order to collect profile in region of interest.  */
> >
> > -  __gthread_mutex_unlock (&__gcov_flush_mx);
> > +void
> > +__gcov_reset (void)
> > +{
> > +  gcov_clear ();
> > +  /* Re-enable dumping to support collecting profile in multiple regions
> > +     of interest.  */
> > +  gcov_dump_complete = 0;
> >  }
> >
> > +/* Function that can be called from application to write profile collected
> > +   so far, in order to collect profile in region of interest.  */
> > +
> > +void
> > +__gcov_dump (void)
> > +{
> > +  gcov_exit ();
> > +  /* Prevent profile from being dumped a second time on application exit.  */
> > +  gcov_dump_complete = 1;
> > +}
> > +
> >  #endif /* L_gcov */
> >
> >  #ifdef L_gcov_merge_add
> > Index: gcc/gcov-io.h
> > ===================================================================
> > --- gcc/gcov-io.h       (revision 187048)
> > +++ gcc/gcov-io.h       (working copy)
> > @@ -458,6 +458,12 @@ extern void __gcov_init (struct gcov_info *) ATTRI
> >  /* Called before fork, to avoid double counting.  */
> >  extern void __gcov_flush (void) ATTRIBUTE_HIDDEN;
> >
> > +/* Function to reset all counters to 0.  */
> > +extern void __gcov_reset (void);
> > +
> > +/* Function to enable early write of profile information so far.  */
> > +extern void __gcov_dump (void);
> > +
> >  /* The merge function that just sums the counters.  */
> >  extern void __gcov_merge_add (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
> >
> >
> > --
> > This patch is available for review at http://codereview.appspot.com/6186044

Patch

Index: libgcc/libgcov.c
===================================================================
--- libgcc/libgcov.c	(revision 187048)
+++ libgcc/libgcov.c	(working copy)
@@ -48,6 +48,8 @@  see the files COPYING3 and COPYING.RUNTIME respect
 #ifdef L_gcov
 void __gcov_init (struct gcov_info *p __attribute__ ((unused))) {}
 void __gcov_flush (void) {}
+void __gcov_reset (void) {}
+void __gcov_dump (void) {}
 #endif
 
 #ifdef L_gcov_merge_add
@@ -91,6 +93,9 @@  static struct gcov_info *gcov_list;
 /* Size of the longest file name. */
 static size_t gcov_max_filename = 0;
 
+/* Flag when the profile has already been dumped via __gcov_dump().  */
+static int gcov_dump_complete = 0;
+
 /* Make sure path component of the given FILENAME exists, create
    missing directories. FILENAME must be writable.
    Returns zero on success, or -1 if an error occurred.  */
@@ -286,6 +291,11 @@  gcov_exit (void)
   char *gi_filename, *gi_filename_up;
   gcov_unsigned_t crc32 = 0;
 
+  /* Prevent the counters from being dumped a second time on exit when the
+     application already wrote out the profile using __gcov_dump().  */
+  if (gcov_dump_complete)
+    return;
+
   memset (&all_prg, 0, sizeof (all_prg));
   /* Find the totals for this execution.  */
   memset (&this_prg, 0, sizeof (this_prg));
@@ -679,6 +689,37 @@  gcov_exit (void)
     }
 }
 
+/* Reset all counters to zero.  */
+
+static void
+gcov_clear (void)
+{
+  const struct gcov_info *gi_ptr;
+
+  for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
+    {
+      unsigned f_ix;
+
+      for (f_ix = 0; f_ix < gi_ptr->n_functions; f_ix++)
+	{
+	  unsigned t_ix;
+	  const struct gcov_fn_info *gfi_ptr = gi_ptr->functions[f_ix];
+
+	  if (!gfi_ptr || gfi_ptr->key != gi_ptr)
+	    continue;
+	  const struct gcov_ctr_info *ci_ptr = gfi_ptr->ctrs;
+	  for (t_ix = 0; t_ix != GCOV_COUNTERS; t_ix++)
+	    {
+	      if (!gi_ptr->merge[t_ix])
+		continue;
+	      
+	      memset (ci_ptr->values, 0, sizeof (gcov_type) * ci_ptr->num);
+	      ci_ptr++;
+	    }
+	}
+    }
+}
+
 /* Add a new object file onto the bb chain.  Invoked automatically
    when running an object file's global ctors.  */
 
@@ -730,38 +771,38 @@  init_mx_once (void)
 void
 __gcov_flush (void)
 {
-  const struct gcov_info *gi_ptr;
-
   init_mx_once ();
   __gthread_mutex_lock (&__gcov_flush_mx);
 
   gcov_exit ();
-  for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
-    {
-      unsigned f_ix;
+  gcov_clear ();
 
-      for (f_ix = 0; f_ix < gi_ptr->n_functions; f_ix++)
-	{
-	  unsigned t_ix;
-	  const struct gcov_fn_info *gfi_ptr = gi_ptr->functions[f_ix];
+  __gthread_mutex_unlock (&__gcov_flush_mx);
+}
 
-	  if (!gfi_ptr || gfi_ptr->key != gi_ptr)
-	    continue;
-	  const struct gcov_ctr_info *ci_ptr = gfi_ptr->ctrs;
-	  for (t_ix = 0; t_ix != GCOV_COUNTERS; t_ix++)
-	    {
-	      if (!gi_ptr->merge[t_ix])
-		continue;
-	      
-	      memset (ci_ptr->values, 0, sizeof (gcov_type) * ci_ptr->num);
-	      ci_ptr++;
-	    }
-	}
-    }
+/* Function that can be called from application to reset counters to zero,
+   in order to collect profile in region of interest.  */
 
-  __gthread_mutex_unlock (&__gcov_flush_mx);
+void
+__gcov_reset (void)
+{
+  gcov_clear ();
+  /* Re-enable dumping to support collecting profile in multiple regions
+     of interest.  */
+  gcov_dump_complete = 0;
 }
 
+/* Function that can be called from application to write profile collected
+   so far, in order to collect profile in region of interest.  */
+
+void
+__gcov_dump (void)
+{
+  gcov_exit ();
+  /* Prevent profile from being dumped a second time on application exit.  */
+  gcov_dump_complete = 1;
+}
+
 #endif /* L_gcov */
 
 #ifdef L_gcov_merge_add
Index: gcc/gcov-io.h
===================================================================
--- gcc/gcov-io.h	(revision 187048)
+++ gcc/gcov-io.h	(working copy)
@@ -458,6 +458,12 @@  extern void __gcov_init (struct gcov_info *) ATTRI
 /* Called before fork, to avoid double counting.  */
 extern void __gcov_flush (void) ATTRIBUTE_HIDDEN;
 
+/* Function to reset all counters to 0.  */
+extern void __gcov_reset (void);
+
+/* Function to enable early write of profile information so far.  */
+extern void __gcov_dump (void);
+
 /* The merge function that just sums the counters.  */
 extern void __gcov_merge_add (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;