public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* a new libgcov interface: __gcov_dump_all
@ 2014-07-18 21:59 Xinliang David Li
  2014-07-20 20:26 ` Nathan Sidwell
  0 siblings, 1 reply; 7+ messages in thread
From: Xinliang David Li @ 2014-07-18 21:59 UTC (permalink / raw)
  To: GCC Patches; +Cc: Jan Hubicka

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

Hi, the following patch implements a new dumper interface to allow
dumping of profile data for all instrumented shared libraries.

For good reasons, existing libgcov implements the dumping on a
per-shared library basis (i.e., gcov_exit is hidden, gcov_list is file
static). This allows each shared library's profile data to be dumped
independently with separate summary data. The downside is that there
is no interface that can be invoked to dump profile data for all
shared modules.

The attached patch does that. Ok for trunk after testing?

thanks,

David

[-- Attachment #2: dump_all.txt --]
[-- Type: text/plain, Size: 3259 bytes --]

Index: libgcc/ChangeLog
===================================================================
--- libgcc/ChangeLog	(revision 212682)
+++ libgcc/ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2014-07-18  Xinliang David Li  <davidxl@google.com>
+
+	* libgcov-driver.c: Force __gcov_dump to be exported
+	* libgcov-interface.c (register_dumper): New function.
+	(__gcov_dump_all): Ditto.
+
 2014-07-14  Richard Biener  <rguenther@suse.de>
 
 	* libgcov.h (struct gcov_fn_info): Make ctrs size 1.
Index: libgcc/libgcov-driver.c
===================================================================
--- libgcc/libgcov-driver.c	(revision 212682)
+++ libgcc/libgcov-driver.c	(working copy)
@@ -55,6 +55,13 @@ extern void set_gcov_dump_complete (void
 extern void reset_gcov_dump_complete (void) ATTRIBUTE_HIDDEN;
 extern int get_gcov_dump_complete (void) ATTRIBUTE_HIDDEN;
 extern void set_gcov_list (struct gcov_info *) ATTRIBUTE_HIDDEN;
+  
+#ifndef IN_GCOV_TOOL
+
+/* Creates strong reference to force _gcov_dump.o to be linked
+   in shared library for exported interfaces.  */
+void (*__gcov_dummy_ref)(void) = &__gcov_dump;
+#endif
 
 struct gcov_fn_buffer
 {
Index: libgcc/libgcov-interface.c
===================================================================
--- libgcc/libgcov-interface.c	(revision 212682)
+++ libgcc/libgcov-interface.c	(working copy)
@@ -115,6 +115,43 @@ __gcov_dump (void)
   set_gcov_dump_complete ();
 }
 
+typedef void (*gcov_dumper_type) (void);
+struct dumper_entry
+{
+  gcov_dumper_type dumper;
+  struct dumper_entry *next_dumper;
+};
+
+static struct dumper_entry this_dumper = {&__gcov_dump, 0};
+
+/* global dumper list with default visibilty. */
+struct dumper_entry *__gcov_dumper_list;
+
+
+__attribute__((constructor))
+static void 
+register_dumper (void)
+{
+  this_dumper.next_dumper = __gcov_dumper_list;
+  __gcov_dumper_list = &this_dumper;
+}
+
+/* Public interface to dump profile data for all shared libraries
+   via registered dumpers from the libraries. This interface
+   has default visibility (unlike gcov_dump which has hidden
+   visbility.  */
+
+void
+__gcov_dump_all (void)
+{
+  struct dumper_entry *dumper = __gcov_dumper_list;
+  while (dumper)
+   {
+     dumper->dumper ();
+     dumper = dumper->next_dumper;
+   }
+}
+
 #endif /* L_gcov_dump */
 
 
Index: libgcc/libgcov.h
===================================================================
--- libgcc/libgcov.h	(revision 212682)
+++ libgcc/libgcov.h	(working copy)
@@ -218,8 +218,14 @@ extern void __gcov_flush (void) ATTRIBUT
 /* 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);
+/* Function to enable early write of profile information so far.  
+   __GCOV_DUMP is also used by __GCOV_DUMP_ALL. The latter 
+   depends on __GCOV_DUMP to have hidden or protected visibility
+   so that each library has its own copy of the registered dumper.  */
+extern void __gcov_dump (void) ATTRIBUTE_HIDDEN;
+
+/* Call __gcov_dump registered from each shared library.  */
+void __gcov_dump_all (void);
 
 /* The merge function that just sums the counters.  */
 extern void __gcov_merge_add (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: a new libgcov interface: __gcov_dump_all
  2014-07-18 21:59 a new libgcov interface: __gcov_dump_all Xinliang David Li
@ 2014-07-20 20:26 ` Nathan Sidwell
  2014-07-20 20:43   ` Xinliang David Li
  0 siblings, 1 reply; 7+ messages in thread
From: Nathan Sidwell @ 2014-07-20 20:26 UTC (permalink / raw)
  To: Xinliang David Li, GCC Patches; +Cc: Jan Hubicka

On 07/18/14 22:41, Xinliang David Li wrote:
> Hi, the following patch implements a new dumper interface to allow
> dumping of profile data for all instrumented shared libraries.
>
> For good reasons, existing libgcov implements the dumping on a
> per-shared library basis (i.e., gcov_exit is hidden, gcov_list is file
> static). This allows each shared library's profile data to be dumped
> independently with separate summary data. The downside is that there
> is no interface that can be invoked to dump profile data for all
> shared modules.

This seems like useful functionality, but I don't think this is the right way to 
do this.  You're duplicating the gcov info object chain.  Why can't you expose 
gcov_list from gcov-driver.c (possibly with a different name, of course?

nathan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: a new libgcov interface: __gcov_dump_all
  2014-07-20 20:26 ` Nathan Sidwell
@ 2014-07-20 20:43   ` Xinliang David Li
  2014-07-21  6:22     ` Nathan Sidwell
  0 siblings, 1 reply; 7+ messages in thread
From: Xinliang David Li @ 2014-07-20 20:43 UTC (permalink / raw)
  To: Nathan Sidwell; +Cc: GCC Patches, Jan Hubicka

The gcov_info chain is not duplicated -- there is already one chain
(linking only modules of the library) per shared library in current
implementation.  My change does not affect underlying behavior at all
-- it merely introduces a new interface to access private dumper
methods associated with shared libs.

David



On Sun, Jul 20, 2014 at 12:42 PM, Nathan Sidwell <nathan@acm.org> wrote:
> On 07/18/14 22:41, Xinliang David Li wrote:
>>
>> Hi, the following patch implements a new dumper interface to allow
>> dumping of profile data for all instrumented shared libraries.
>>
>> For good reasons, existing libgcov implements the dumping on a
>> per-shared library basis (i.e., gcov_exit is hidden, gcov_list is file
>> static). This allows each shared library's profile data to be dumped
>> independently with separate summary data. The downside is that there
>> is no interface that can be invoked to dump profile data for all
>> shared modules.
>
>
> This seems like useful functionality, but I don't think this is the right
> way to do this.  You're duplicating the gcov info object chain.  Why can't
> you expose gcov_list from gcov-driver.c (possibly with a different name, of
> course?
>
> nathan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: a new libgcov interface: __gcov_dump_all
  2014-07-20 20:43   ` Xinliang David Li
@ 2014-07-21  6:22     ` Nathan Sidwell
  2014-07-22 16:29       ` Xinliang David Li
  0 siblings, 1 reply; 7+ messages in thread
From: Nathan Sidwell @ 2014-07-21  6:22 UTC (permalink / raw)
  To: Xinliang David Li; +Cc: GCC Patches, Jan Hubicka

On 07/20/14 21:38, Xinliang David Li wrote:
> The gcov_info chain is not duplicated -- there is already one chain
> (linking only modules of the library) per shared library in current
> implementation.  My change does not affect underlying behavior at all
> -- it merely introduces a new interface to access private dumper
> methods associated with shared libs.

ah, got it.  thanks for clarifying.  Can't help thinking gcov_init should be 
doing this, and wondering about dlload/dlclose.  Let me think

nathan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: a new libgcov interface: __gcov_dump_all
  2014-07-21  6:22     ` Nathan Sidwell
@ 2014-07-22 16:29       ` Xinliang David Li
  2017-10-26  9:01         ` Martin Liška
  0 siblings, 1 reply; 7+ messages in thread
From: Xinliang David Li @ 2014-07-22 16:29 UTC (permalink / raw)
  To: Nathan Sidwell; +Cc: GCC Patches, Jan Hubicka

[-- Attachment #1: Type: text/plain, Size: 780 bytes --]

Please take a look the updated patch. It addresses the issue of using
dlclose before dump, and potential races (between a thread closing a
library and the dumper call).

David

On Sun, Jul 20, 2014 at 11:12 PM, Nathan Sidwell <nathan@acm.org> wrote:
> On 07/20/14 21:38, Xinliang David Li wrote:
>>
>> The gcov_info chain is not duplicated -- there is already one chain
>> (linking only modules of the library) per shared library in current
>> implementation.  My change does not affect underlying behavior at all
>> -- it merely introduces a new interface to access private dumper
>> methods associated with shared libs.
>
>
> ah, got it.  thanks for clarifying.  Can't help thinking gcov_init should be
> doing this, and wondering about dlload/dlclose.  Let me think
>
> nathan

[-- Attachment #2: dump_all.txt --]
[-- Type: text/plain, Size: 4560 bytes --]

Index: libgcc/libgcov.h
===================================================================
--- libgcc/libgcov.h	(revision 212682)
+++ libgcc/libgcov.h	(working copy)
@@ -218,8 +218,14 @@ extern void __gcov_flush (void) ATTRIBUT
 /* 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);
+/* Function to enable early write of profile information so far.  
+   __GCOV_DUMP is also used by __GCOV_DUMP_ALL. The latter 
+   depends on __GCOV_DUMP to have hidden or protected visibility
+   so that each library has its own copy of the registered dumper.  */
+extern void __gcov_dump (void) ATTRIBUTE_HIDDEN;
+
+/* Call __gcov_dump registered from each shared library.  */
+void __gcov_dump_all (void);
 
 /* The merge function that just sums the counters.  */
 extern void __gcov_merge_add (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
Index: libgcc/ChangeLog
===================================================================
--- libgcc/ChangeLog	(revision 212682)
+++ libgcc/ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2014-07-18  Xinliang David Li  <davidxl@google.com>
+
+	* libgcov-driver.c: Force __gcov_dump to be exported
+	* libgcov-interface.c (register_dumper): New function.
+	(__gcov_dump_all): Ditto.
+
 2014-07-14  Richard Biener  <rguenther@suse.de>
 
 	* libgcov.h (struct gcov_fn_info): Make ctrs size 1.
Index: libgcc/libgcov-driver.c
===================================================================
--- libgcc/libgcov-driver.c	(revision 212682)
+++ libgcc/libgcov-driver.c	(working copy)
@@ -55,6 +55,13 @@ extern void set_gcov_dump_complete (void
 extern void reset_gcov_dump_complete (void) ATTRIBUTE_HIDDEN;
 extern int get_gcov_dump_complete (void) ATTRIBUTE_HIDDEN;
 extern void set_gcov_list (struct gcov_info *) ATTRIBUTE_HIDDEN;
+  
+#ifndef IN_GCOV_TOOL
+
+/* Creates strong reference to force _gcov_dump.o to be linked
+   in shared library for exported interfaces.  */
+void (*__gcov_dummy_ref)(void) = &__gcov_dump;
+#endif
 
 struct gcov_fn_buffer
 {
Index: libgcc/libgcov-interface.c
===================================================================
--- libgcc/libgcov-interface.c	(revision 212682)
+++ libgcc/libgcov-interface.c	(working copy)
@@ -115,6 +115,100 @@ __gcov_dump (void)
   set_gcov_dump_complete ();
 }
 
+typedef void (*gcov_dumper_type) (void);
+struct dumper_entry
+{
+  gcov_dumper_type dumper;
+  struct dumper_entry *next_dumper;
+};
+
+static struct dumper_entry this_dumper = {&__gcov_dump, 0};
+
+/* global dumper list with default visibilty. */
+struct dumper_entry *__gcov_dumper_list;
+
+#ifdef __GTHREAD_MUTEX_INIT
+__gthread_mutex_t __gcov_dump_mx = __GTHREAD_MUTEX_INIT;
+#define init_mx_once()
+#else
+__gthread_mutex_t __gcov_dump_mx;
+
+static void
+init_mx (void)
+{
+  __GTHREAD_MUTEX_INIT_FUNCTION (&__gcov_dump_mx);
+}
+static void
+init_mx_once (void)
+{
+  static __gthread_once_t once = __GTHREAD_ONCE_INIT;
+  __gthread_once (&once, init_mx);
+}
+#endif
+
+/* Register the library private __gcov_dump method
+   to the global list.  */
+
+__attribute__((constructor))
+static void
+register_dumper (void)
+{
+  init_mx_once ();
+  __gthread_mutex_lock (&__gcov_dump_mx);
+  this_dumper.next_dumper = __gcov_dumper_list;
+  __gcov_dumper_list = &this_dumper;
+  __gthread_mutex_unlock (&__gcov_dump_mx);
+}
+
+__attribute__((destructor))
+static void
+unregister_dumper (void)
+{
+  struct dumper_entry *dumper;
+  struct dumper_entry *prev_dumper = 0;
+
+  init_mx_once ();
+  __gthread_mutex_lock (&__gcov_dump_mx);
+  dumper = __gcov_dumper_list;
+
+  while (dumper)
+    {
+      if (dumper->dumper == &__gcov_dump)
+        {
+	  if (prev_dumper)
+	    prev_dumper->next_dumper = dumper->next_dumper;
+ 	  else
+	    __gcov_dumper_list = dumper->next_dumper;
+          break;
+        }
+      prev_dumper = dumper;
+      dumper = dumper->next_dumper;
+    }
+  __gthread_mutex_unlock (&__gcov_dump_mx);
+}
+
+/* Public interface to dump profile data for all shared libraries
+   via registered dumpers from the libraries. This interface
+   has default visibility (unlike gcov_dump which has hidden
+   visbility.  */
+
+void
+__gcov_dump_all (void)
+{
+  struct dumper_entry *dumper;
+
+  init_mx_once ();
+  __gthread_mutex_lock (&__gcov_dump_mx);
+
+  dumper = __gcov_dumper_list;
+  while (dumper)
+   {
+     dumper->dumper ();
+     dumper = dumper->next_dumper;
+   }
+  __gthread_mutex_unlock (&__gcov_dump_mx);
+}
+
 #endif /* L_gcov_dump */
 
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: a new libgcov interface: __gcov_dump_all
  2014-07-22 16:29       ` Xinliang David Li
@ 2017-10-26  9:01         ` Martin Liška
  2017-12-29 14:17           ` Martin Liška
  0 siblings, 1 reply; 7+ messages in thread
From: Martin Liška @ 2017-10-26  9:01 UTC (permalink / raw)
  To: Xinliang David Li, Nathan Sidwell; +Cc: GCC Patches, Jan Hubicka

On 07/22/2014 06:04 PM, Xinliang David Li wrote:
> Please take a look the updated patch. It addresses the issue of using
> dlclose before dump, and potential races (between a thread closing a
> library and the dumper call).
> 
> David
> 
> On Sun, Jul 20, 2014 at 11:12 PM, Nathan Sidwell <nathan@acm.org> wrote:
>> On 07/20/14 21:38, Xinliang David Li wrote:
>>>
>>> The gcov_info chain is not duplicated -- there is already one chain
>>> (linking only modules of the library) per shared library in current
>>> implementation.  My change does not affect underlying behavior at all
>>> -- it merely introduces a new interface to access private dumper
>>> methods associated with shared libs.
>>
>>
>> ah, got it.  thanks for clarifying.  Can't help thinking gcov_init should be
>> doing this, and wondering about dlload/dlclose.  Let me think
>>
>> nathan

Hi.

Unfortunately, it looks the patch hasn't been installed to trunk. Some folks from Firefox
are interested in that.

Are you Nathan OK with the patch? I guess a rebase will be needed.

Martin

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: a new libgcov interface: __gcov_dump_all
  2017-10-26  9:01         ` Martin Liška
@ 2017-12-29 14:17           ` Martin Liška
  0 siblings, 0 replies; 7+ messages in thread
From: Martin Liška @ 2017-12-29 14:17 UTC (permalink / raw)
  To: Xinliang David Li, Nathan Sidwell; +Cc: GCC Patches, Jan Hubicka

On 10/26/2017 10:47 AM, Martin Liška wrote:
> On 07/22/2014 06:04 PM, Xinliang David Li wrote:
>> Please take a look the updated patch. It addresses the issue of using
>> dlclose before dump, and potential races (between a thread closing a
>> library and the dumper call).
>>
>> David
>>
>> On Sun, Jul 20, 2014 at 11:12 PM, Nathan Sidwell <nathan@acm.org> wrote:
>>> On 07/20/14 21:38, Xinliang David Li wrote:
>>>>
>>>> The gcov_info chain is not duplicated -- there is already one chain
>>>> (linking only modules of the library) per shared library in current
>>>> implementation.  My change does not affect underlying behavior at all
>>>> -- it merely introduces a new interface to access private dumper
>>>> methods associated with shared libs.
>>>
>>>
>>> ah, got it.  thanks for clarifying.  Can't help thinking gcov_init should be
>>> doing this, and wondering about dlload/dlclose.  Let me think
>>>
>>> nathan
> 
> Hi.
> 
> Unfortunately, it looks the patch hasn't been installed to trunk. Some folks from Firefox
> are interested in that.
> 
> Are you Nathan OK with the patch? I guess a rebase will be needed.
> 
> Martin
> 

Hi.

I would like to suspend the patch review as there's actually no call for the feature.
The person which sent me the link actually wanted to dump all counters. That can be
easily done via __gcov_dump interface we already have.

Martin

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-12-29 14:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-18 21:59 a new libgcov interface: __gcov_dump_all Xinliang David Li
2014-07-20 20:26 ` Nathan Sidwell
2014-07-20 20:43   ` Xinliang David Li
2014-07-21  6:22     ` Nathan Sidwell
2014-07-22 16:29       ` Xinliang David Li
2017-10-26  9:01         ` Martin Liška
2017-12-29 14:17           ` Martin Liška

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).