[RFC] libgcov.c re-factoring and offline profile-tool
Xinliang David Li
davidxl@google.com
Tue Nov 5 00:22:00 GMT 2013
I wonder if it makes sense to get rid of the conditional compile such as
#ifdef L_gcov_xxxxx
..
#endif
This has the advantage of producing slightly smaller instrumented
binary, but this benefit can also be achieved via -ffunction-sections
and let linker to garbage collect unused functions.
(probably as a follow up if it makes sense).
David
On Mon, Nov 4, 2013 at 2:43 PM, Rong Xu <xur@google.com> wrote:
> Honza, Thanks for the comments!
> Attached is the patch.
> Passed spec2006 fdo build and profiledbootstrap in gcc.
> I'm doing some other tests.
>
> -Rong
>
> On Mon, Nov 4, 2013 at 1:55 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> Thanks! Can you attach the patch file for the proposed change?
>>>
>>> David
>>>
>>> On Fri, Nov 1, 2013 at 2:08 PM, Rong Xu <xur@google.com> wrote:
>>> > libgcov.c is the main file to generate libgcov.a. This single source generates
>>> > 21 object files and then archives to a library. These objects are of
>>> > very different purposes but they are in the same file guarded by various macros.
>>> > The source file becomes quite large and its readability becomes very
>>> > low. In its current state:
>>> > 1) The code is very hard to understand by new developers;
>>
>> Agreed, libgcov has became hard to maintain since several parts are written
>> in very low-level way.
>>> > 2) It makes regressions more likely when making simple changes;
>>> > More importantly,
>>> > 3) it makes code reuse/sharing very difficult. For instance, Using
>>> > libgcov to implement an offline profile tool (see below); kernel FDO
>>> > support etc.
>>
>> Yep, it was my longer term plan to do this, too.
>>> >
>>> > We came to this issue when working on an offline tool to handle
>>> > profile counters.
>>> > Our plan is to have a profile-tool can merge, diff, collect statistics, and
>>> > better dump raw profile counters. This is one of the most requested features
>>> > from out internal users. This tool can also be very useful for any FDO/coverage
>>> > users. In the first part of tool, we want to have the weight
>>> > merge. Again, to reuse the code, we have to change libgcov.c. But we don't want
>>> > to further diverge libgcov.a in our google branches from the trunk.
>>> >
>>> > Another issue in libgcov.c is function gcov_exit(). It is a huge function
>>> > with about 467 lines of code. This function traverses gcov_list and dumps all
>>> > the gcda files. The code for merging the gcda files also sits in the same
>>> > function. The structure makes the code-reuse and extension really difficult
>>> > (like in kernel FDO, we have to break this function to reuse some of the code).
>>> >
>>> > We propose to break gcov_exit into smaller functions and split libgcov.c into
>>> > several files. Our plan for profile-tool is listed in (3).
>>> >
>>> > 1. break gcov_exit() into the following structure:
>>> > gcov_exit()
>>> > --> gcov_exit_compute_summary()
>>> > --> allocate_filename_struct()
>>> > for gi_ptr in gcov_list
>>> > --> gcov_exit_dump_gcov()
>>> > --> gcov_exit_open_gcda_file()
>>> > --> gcov_exit_merge_gcda ()
>>> > --> gcov_exit_write_gcda ()
>>
>> Just a short comment that summaries are also produced during the merging - i.e. they are
>> done on per-file basis. But otherwise I agree.
>> We should be cureful to not increase footprint of libgcov much for embedded users, but
>> I believe changes like this can be done w/o additional overhead in the optimized library.
>
> The merge of summary has two parts. One is to find the summary to
> merge to. That is in gcov_exit_merge_gcda().
> The other part of do the actual merging of summary is in gcov_exit_dump_gcov().
>
> I tried to keep the current work flow. So except for a few more calls,
> I don't think there is much overhead.
>
>>> >
>>> > 2. split libgcov.c into the following files:
>>> > libgcov-profiler.c
>>> > libgcov-merge.c
>>> > libgcov-interface.c
>>> > libgcov-driver.c
>>> > libgcov-driver-system.c (source included into libgcov-driver.c)
>>
>> Seems resonable. Splitting i/o stuff into separate module (driver) is something I planned
>> for long time. What is difference in between libgcov-interface and libgcov-driver?
>
> Basically libgcov-driver handles all the dumping of gcov-info.
> libgcov-interface is the API used by instrumentation, like the wrapper for fork.
>
>
>>> >
>>> > The details of the splitting:
>>> > libgcov-interface.c
>>> > /* This file contains API functions to other programs and the supporting
>>> > functions. */
>>> > __gcov_dump()
>>> > __gcov_execl()
>>> > __gcov_execle()
>>> > __gcov_execlp()
>>> > __gcov_execv()
>>> > __gcov_execve()
>>> > __gcov_execvp()
>>> > __gcov_flush()
>>> > __gcov_fork()
>>> > __gcov_reset()
>>> > init_mx()
>>> > init_mx_once()
>>> >
>>> > libgcov-profiler.c
>>> > /* This file contains runtime profile handlers. */
>>> > variables:
>>> > __gcov_indirect_call_callee
>>> > __gcov_indirect_call_counters
>>> > functions:
>>> > __gcov_average_profiler()
>>> > __gcov_indirect_call_profiler()
>>> > __gcov_indirect_call_profiler_v2()
>>> > __gcov_interval_profiler()
>>> > __gcov_ior_profiler()
>>> > __gcov_one_value_profiler()
>>> > __gcov_one_value_profiler_body()
>>> > __gcov_pow2_profiler()
>>> >
>>> > libgcov-merge.c
>>> > /* This file contains the merge functions for various counters. */
>>> > functions:
>>> > __gcov_merge_add()
>>> > __gcov_merge_delta()
>>> > __gcov_merge_ior()
>>> > __gcov_merge_single()
>>> >
>>> > libcov-driver.c
>>> > /* This file contains the gcov dumping functions. We separate the
>>> > system dependent part to libgcov-driver-system.c. */
>>> > variables:
>>> > gcov_list
>>> > gcov_max_filename
>>> > gcov_dump_complete
>>> > ------ newly added file static variables --
>>> > this_prg
>>> > all_prg
>>> > crc32
>>> > gi_filename
>>> > fn_buffer
>>> > fn_tail
>>> > sum_buffer
>>> > next_sum_buffer
>>> > sum_tail
>>> > ----- end -----
>>> > functions:
>>> > free_fn_data()
>>> > buffer_fn_data()
>>> > crc32_unsigned()
>>> > gcov_version()
>>> > gcov_histogram_insert()
>>> > gcov_compute_histogram()
>>> > gcov_clear()
>>> > __gcov_init()
>>> > gcov_exit()
>>> > ------- newly added static functions --
>>> > gcov_exit_compute_summary ()
>>> > gcov_exit_merge_gcda()
>>> > gcov_exit_write_gcda()
>>> > gcov_exit_dump_gcov()
>>> > ----- end -----
>>> >
>>> > libgcov-driver-system.c
>>> > /* system dependent part of ligcov-driver.c. */
>>> > functions:
>>> > create_file_directory()
>>> > ------- newly added static functions --
>>> > gcov_error() /* This replaces all fprintf(stderr, ...) */
>>> > allocate_filename_struct()
>>> > gcov_exit_open_gcda_file()
>>> >
>>> > 3. add offline profile-tool support.
>>> > We will change the interface of merge functions to make it
>>> > take in-memory gcov_info list, and a weight as the arguments.
>>> >
>>> > We will add libgcov-tool.c. It has two APIs:
>>> > (1) read_profile_dir(): read a profile directory and reconstruct the
>>> > gcov_info link list in-memory.
>>> > (2) merge_profiles(): merge two in-memory gcov_info link list.
>>> >
>>> > We also add profile-tool.c in gcc directory. It will source-include
>>> > libgcov-tool.c and build a host binary. (We don't do library style
>>> > because that will need a hard dependence from gcc to libgcc).
>>> > profile-tool.c will mainly handle user options and the write-out of
>>> > gcov-info link list. Some changes in gcov-io.[ch] will be also needed.
>>> >
>>> > Thanks,
>>
>> Thanks, it all looks good to me. One thing I also always wondered about is why
>> libgcov ended up being partly in libgcc and partly in gcc (the headers) directory.
>> It would make more sense to have libgcov directory like we have for other runtime
>> libraries.
>>
> I agree with you -- it's better to have a seperate directoy for libgcov.
>
> The other part should be re-refatored is the gcov-io.[ch]. I think they
> are even messier than libgcov.c. I plan to clean them after libgcvo.c.
>
>> Honza
>>> >
>>> > -Rong
More information about the Gcc-patches
mailing list