Here is the patch that addresses Honza's concern about bss increment.
It just makes this_prg a local variable.
Some comments are inlined.
On Fri, Dec 6, 2013 at 6:23 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
> ....
> Do you know how the size of libgcov changed with your patch?
> Quick check of current mainline on compiling empty main gives:
>
> jh@gcc10:~/trunk/build/gcc$ cat t.c
> main()
> {
> }
> jh@gcc10:~/trunk/build/gcc$ ./xgcc -B ./ -O2 -fprofile-generate -o a.out-new
> --static t.c
> jh@gcc10:~/trunk/build/gcc$ gcc -O2 -fprofile-generate -o a.out-old --static
> t.c
> jh@gcc10:~/trunk/build/gcc$ size a.out-old
> text data bss dec hex filename
> 608141 3560 16728 628429 996cd a.out-old
> jh@gcc10:~/trunk/build/gcc$ size a.out-new
> text data bss dec hex filename
> 612621 3688 22880 639189 9c0d5 a.out-new
>
> Without profiling I get:
> jh@gcc10:~/trunk/build/gcc$ size a.out-new-no
> jh@gcc10:~/trunk/build/gcc$ size a.out-old-no
> text data bss dec hex filename
> 599719 3448 12568 615735 96537 a.out-old-no
> text data bss dec hex filename
> 600247 3448 12568 616263 96747 a.out-new-no
>
> Quite big for empty program, but mostly glibc fault, I suppose
> (that won't be an issue for embedded platforms). But anyway
> we increased text size overhead from 8k to 12k, BSS size
> overhead from 4k to 10k and data by another 1k.
>
I think it would more fair to compare r204729 and r204730. Your
comparison had some other changes in libgcov such as time_profiler and
indirecto_call_profiler_v2.
Using the same empty t.c, for r204729, we have
xur2%208:gcc >> ./xgcc -B ./ -O2 -fprofile-generate --static -o
a.out-r204729 t.c
xur2%209:gcc >> size a.out-r204729
text data bss dec hex filename
803207 6352 15448 825007 c96af a.out-r204729
xur2%210:gcc >> ./xgcc -B ./ -O2 --static -o a.out-r204729-no t.c
xur2%211:gcc >> size a.out-r204729-no
text data bss dec hex filename
790337 6112 11336 807785 c5369 a.out-r204729-no
For r204730, we have
xur2%216:gcc >> ./xgcc -B ./ -O2 -fprofile-generate --static -o
a.out-r204730 t.c
xur2%217:gcc >> size a.out-r204730
text data bss dec hex filename
802919 6384 21592 830895 cadaf a.out-r204730
xur2%218:gcc >> ./xgcc -B ./ -O2 --static -o a.out-r204730-no t.c
xur2%219:gcc >> size a.out-r204730-no
text data bss dec hex filename
790337 6112 11336 807785 c5369 a.out-r204730-no
r204730 actually has smaller text, data size with -fprofile-generate.
You are right about there are 6kb more bss space due to the static
variables introduced. It mostly caused by this_prg object.
With the attached trunk patch that localizes this_prg, we have
xur2%42:fdo >> size a.out-new
text data bss dec hex filename
803479 6456 15512 825447 c9867 a.out-new
xur2%43:fdo >> size a.out-new-no
text data bss dec hex filename
790545 6112 11368 808025 c5459 a.out-new-no
We are now using 64 more bytes in m64.
Objects size for r204730:
text data bss dec hex filename
57 0 0 57 39 _gcov_average_profiler.o
66 0 0 66 42 _gcov_dump.o
516 0 0 516 204 _gcov_execle.o
476 0 0 476 1dc _gcov_execl.o
476 0 0 476 1dc _gcov_execlp.o
108 0 0 108 6c _gcov_execve.o
98 0 0 98 62 _gcov_execv.o
98 0 0 98 62 _gcov_execvp.o
126 0 40 166 a6 _gcov_flush.o
101 0 0 101 65 _gcov_fork.o
122 0 0 122 7a _gcov_indirect_call_profiler.o
178 0 16 194 c2 _gcov_indirect_call_profiler_v2.o
89 0 0 89 59 _gcov_interval_profiler.o
52 0 0 52 34 _gcov_ior_profiler.o
126 0 0 126 7e _gcov_merge_add.o
242 0 0 242 f2 _gcov_merge_delta.o
126 0 0 126 7e _gcov_merge_ior.o
251 0 0 251 fb _gcov_merge_single.o
156 0 0 156 9c _gcov_merge_time_profile.o
9252 0 6144 15396 3c24 _gcov.o
115 0 0 115 73 _gcov_one_value_profiler.o
69 0 0 69 45 _gcov_pow2_profiler.o
66 0 0 66 42 _gcov_reset.o
77 0 8 85 55 _gcov_time_profiler.o
Objects size for r204729:
text data bss dec hex filename
57 0 0 57 39 _gcov_average_profiler.o
72 0 0 72 48 _gcov_dump.o
516 0 0 516 204 _gcov_execle.o
476 0 0 476 1dc _gcov_execl.o
476 0 0 476 1dc _gcov_execlp.o
108 0 0 108 6c _gcov_execve.o
98 0 0 98 62 _gcov_execv.o
98 0 0 98 62 _gcov_execvp.o
101 0 0 101 65 _gcov_fork.o
122 0 0 122 7a _gcov_indirect_call_profiler.o
178 0 16 194 c2 _gcov_indirect_call_profiler_v2.o
89 0 0 89 59 _gcov_interval_profiler.o
52 0 0 52 34 _gcov_ior_profiler.o
126 0 0 126 7e _gcov_merge_add.o
242 0 0 242 f2 _gcov_merge_delta.o
126 0 0 126 7e _gcov_merge_ior.o
251 0 0 251 fb _gcov_merge_single.o
156 0 0 156 9c _gcov_merge_time_profile.o
9564 0 64 9628 259c _gcov.o
115 0 0 115 73 _gcov_one_value_profiler.o
69 0 0 69 45 _gcov_pow2_profiler.o
72 0 0 72 48 _gcov_reset.o
77 0 0 77 4d _gcov_time_profiler.o
> text data bss dec hex filename
> 126 0 0 126 7e _gcov_merge_add.o (ex libgcov.a)
> 251 0 0 251 fb _gcov_merge_single.o (ex libgcov.a)
> 242 0 0 242 f2 _gcov_merge_delta.o (ex libgcov.a)
> 126 0 0 126 7e _gcov_merge_ior.o (ex libgcov.a)
> 156 0 0 156 9c _gcov_merge_time_profile.o (ex
> libgcov.a)
> 89 0 0 89 59 _gcov_interval_profiler.o (ex
> libgcov.a)
> 69 0 0 69 45 _gcov_pow2_profiler.o (ex libgcov.a)
> 115 0 0 115 73 _gcov_one_value_profiler.o (ex
> libgcov.a)
> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex
> libgcov.a)
> 57 0 0 57 39 _gcov_average_profiler.o (ex
> libgcov.a)
> 52 0 0 52 34 _gcov_ior_profiler.o (ex libgcov.a)
> 178 0 16 194 c2 _gcov_indirect_call_profiler_v2.o (ex
> libgcov.a)
> 77 0 8 85 55 _gcov_time_profiler.o (ex libgcov.a)
> 126 0 40 166 a6 _gcov_flush.o (ex libgcov.a)
> 101 0 0 101 65 _gcov_fork.o (ex libgcov.a)
> 471 0 0 471 1d7 _gcov_execl.o (ex libgcov.a)
> 471 0 0 471 1d7 _gcov_execlp.o (ex libgcov.a)
> 524 0 0 524 20c _gcov_execle.o (ex libgcov.a)
> 98 0 0 98 62 _gcov_execv.o (ex libgcov.a)
> 98 0 0 98 62 _gcov_execvp.o (ex libgcov.a)
> 108 0 0 108 6c _gcov_execve.o (ex libgcov.a)
> 66 0 0 66 42 _gcov_reset.o (ex libgcov.a)
> 66 0 0 66 42 _gcov_dump.o (ex libgcov.a)
> 9505 0 6144 15649 3d21 _gcov.o (ex libgcov.a)
>
> I think we definitely need to move those 6k of bss space out. I think those
> are new
> static vars you introduced that I think are unsafe anyway because multiple
> streaming
> may run at once in threaded program where locking mechanizm fails.
> (it will probably do other bad things, but definitely we do not want to
> conflict on things like filename to write into).
I don't understand why static variables can cause any safety issue in
multi-thread programs.
For any process, gcov_dump will be called only once and there will be
one instance of globals.
Where is the problem?
The file locking is for multi-process program. For that case, each
process has its own instance of globals.
I don't see this causes any new problems.
>
> Compared to my system gcov:
> text data bss dec hex filename
> 9765 0 64 9829 2665 _gcov.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 125 0 0 125 7d _gcov_merge_add.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 251 0 0 251 fb _gcov_merge_single.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 242 0 0 242 f2 _gcov_merge_delta.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 101 0 0 101 65 _gcov_fork.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 471 0 0 471 1d7 _gcov_execl.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 471 0 0 471 1d7 _gcov_execlp.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 524 0 0 524 20c _gcov_execle.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 98 0 0 98 62 _gcov_execv.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 98 0 0 98 62 _gcov_execvp.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 108 0 0 108 6c _gcov_execve.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 72 0 0 72 48 _gcov_reset.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 72 0 0 72 48 _gcov_dump.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 89 0 0 89 59 _gcov_interval_profiler.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 69 0 0 69 45 _gcov_pow2_profiler.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 115 0 0 115 73 _gcov_one_value_profiler.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 57 0 0 57 39 _gcov_average_profiler.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 52 0 0 52 34 _gcov_ior_profiler.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
> 125 0 0 125 7d _gcov_merge_ior.o (ex
> ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a)
>
2014-01-08 Rong Xu <x...@google.com>
* libgcc/libgcov-driver.c (this_prg): make it local to save
bss space.
(gcov_exit_compute_summary): Ditto.
(gcov_exit_merge_gcda): Ditto.
(gcov_exit_merge_summary): Ditto.
(gcov_exit_dump_gcov): Ditto.
Index: libgcc/libgcov-driver.c
===================================================================
--- libgcc/libgcov-driver.c (revision 206437)
+++ libgcc/libgcov-driver.c (working copy)
@@ -303,8 +303,6 @@ gcov_compute_histogram (struct gcov_summary *sum)
}
}
-/* summary for program. */
-static struct gcov_summary this_prg;
/* gcda filename. */
static char *gi_filename;
/* buffer for the fn_data from another program. */
@@ -317,10 +315,10 @@ static struct gcov_summary_buffer *sum_buffer;
static int run_accounted = 0;
/* This funtions computes the program level summary and the histo-gram.
- It computes and returns CRC32 and stored summari in THIS_PRG. */
+ It computes and returns CRC32 and stored summary in THIS_PRG. */
static gcov_unsigned_t
-gcov_exit_compute_summary (void)
+gcov_exit_compute_summary (struct gcov_summary *this_prg)
{
struct gcov_info *gi_ptr;
const struct gcov_fn_info *gfi_ptr;
@@ -332,7 +330,7 @@ static gcov_unsigned_t
gcov_unsigned_t crc32 = 0;
/* Find the totals for this execution. */
- memset (&this_prg, 0, sizeof (this_prg));
+ memset (this_prg, 0, sizeof (*this_prg));
for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
{
crc32 = crc32_unsigned (crc32, gi_ptr->stamp);
@@ -357,7 +355,7 @@ static gcov_unsigned_t
if (!gi_ptr->merge[t_ix])
continue;
- cs_ptr = &this_prg.ctrs[t_ix];
+ cs_ptr = &(this_prg->ctrs[t_ix]);
cs_ptr->num += ci_ptr->num;
crc32 = crc32_unsigned (crc32, ci_ptr->num);
@@ -371,7 +369,7 @@ static gcov_unsigned_t
}
}
}
- gcov_compute_histogram (&this_prg);
+ gcov_compute_histogram (this_prg);
return crc32;
}
@@ -393,6 +391,7 @@ struct gcov_filename_aux{
static int
gcov_exit_merge_gcda (struct gcov_info *gi_ptr,
struct gcov_summary *prg_p,
+ struct gcov_summary *this_prg,
gcov_position_t *summary_pos_p,
gcov_position_t *eof_pos_p,
gcov_unsigned_t crc32)
@@ -446,7 +445,7 @@ gcov_exit_merge_gcda (struct gcov_info *gi_ptr,
goto next_summary;
for (t_ix = 0; t_ix != GCOV_COUNTERS_SUMMABLE; t_ix++)
- if (tmp.ctrs[t_ix].num != this_prg.ctrs[t_ix].num)
+ if (tmp.ctrs[t_ix].num != this_prg->ctrs[t_ix].num)
goto next_summary;
*prg_p = tmp;
*summary_pos_p = *eof_pos_p;
@@ -636,7 +635,7 @@ gcov_exit_write_gcda (const struct gcov_info *gi_p
static int
gcov_exit_merge_summary (const struct gcov_info *gi_ptr, struct gcov_summary
*prg,
- gcov_unsigned_t crc32,
+ struct gcov_summary *this_prg, gcov_unsigned_t crc32,
struct gcov_summary *all_prg __attribute__ ((unused)))
{
struct gcov_ctr_summary *cs_prg, *cs_tprg;
@@ -650,7 +649,7 @@ gcov_exit_merge_summary (const struct gcov_info *g
for (t_ix = 0; t_ix < GCOV_COUNTERS_SUMMABLE; t_ix++)
{
cs_prg = &(prg->ctrs[t_ix]);
- cs_tprg = &this_prg.ctrs[t_ix];
+ cs_tprg = &(this_prg->ctrs[t_ix]);
if (gi_ptr->merge[t_ix])
{
@@ -719,7 +718,8 @@ gcov_exit_merge_summary (const struct gcov_info *g
static void
gcov_exit_dump_gcov (struct gcov_info *gi_ptr, struct gcov_filename_aux *gf,
- gcov_unsigned_t crc32, struct gcov_summary *all_prg)
+ gcov_unsigned_t crc32, struct gcov_summary *all_prg,
+ struct gcov_summary *this_prg)
{
struct gcov_summary prg; /* summary for this object over all program. */
int error;
@@ -743,7 +743,7 @@ gcov_exit_dump_gcov (struct gcov_info *gi_ptr, str
gcov_error ("profiling:%s:Not a gcov data file\n", gi_filename);
goto read_fatal;
}
- error = gcov_exit_merge_gcda (gi_ptr, &prg, &summary_pos, &eof_pos,
+ error = gcov_exit_merge_gcda (gi_ptr, &prg, this_prg, &summary_pos,
&eof_pos,
crc32);
if (error == -1)
goto read_fatal;
@@ -757,7 +757,7 @@ gcov_exit_dump_gcov (struct gcov_info *gi_ptr, str
summary_pos = eof_pos;
}
- error = gcov_exit_merge_summary (gi_ptr, &prg, crc32, all_prg);
+ error = gcov_exit_merge_summary (gi_ptr, &prg, this_prg, crc32, all_prg);
if (error == -1)
goto read_fatal;
@@ -787,13 +787,14 @@ gcov_exit (void)
struct gcov_filename_aux gf;
gcov_unsigned_t crc32;
struct gcov_summary all_prg;
+ struct gcov_summary this_prg;
/* 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;
- crc32 = gcov_exit_compute_summary ();
+ crc32 = gcov_exit_compute_summary (&this_prg);
allocate_filename_struct (&gf);
#if !GCOV_LOCKED
@@ -802,7 +803,7 @@ gcov_exit (void)
/* Now merge each file. */
for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
- gcov_exit_dump_gcov (gi_ptr, &gf, crc32, &all_prg);
+ gcov_exit_dump_gcov (gi_ptr, &gf, crc32, &all_prg, &this_prg);
run_accounted = 1;
if (gi_filename)
