[google]Add support for sampled profile collection (issue4438083)

[Easwaran Raman]

I want to revive this patch for mainline and have some questions on
Honza's comments.

On Fri, Apr 29, 2011 at 1:48 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>
>> A known limitation is that value profiling is not yet sampled, but it
>> does not seem to cause problems.
>
> For the performance alone, we probably don't need to care that much
> given the fact that we value porfile only relatively expensive operations.
> But if we want to have the turn off/on feature, then i gueess we need to
> guard everything.  It is not much of pain to add the code generating
> conditionals everywhere, after all.

If we sample value profiling instrumentation as well, does it make
sense to use a single counter and rate for all instrumentations. If
not, does the additional complexity (and flags) justify the benefit of
uniformity?

>> > +/* Insert STMT_IF around given sequence of consecutive statements in the
>> > +   same basic block starting with STMT_START, ending with STMT_END.  */
>> > +
>> > +static void
>> > +insert_if_then (gimple stmt_start, gimple stmt_end, gimple stmt_if)
>> > +{
>> > +  gimple_stmt_iterator gsi;
>> > +  basic_block bb_original, bb_before_if, bb_after_if;
>> > +  edge e_if_taken, e_then_join;
>> > +
>> > +  gsi = gsi_for_stmt (stmt_start);
>> > +  gsi_insert_before (&gsi, stmt_if, GSI_SAME_STMT);
>> > +  bb_original = gsi_bb (gsi);
>> > +  e_if_taken = split_block (bb_original, stmt_if);
>> > +  e_if_taken->flags &= ~EDGE_FALLTHRU;
>> > +  e_if_taken->flags |= EDGE_TRUE_VALUE;
>> > +  e_then_join = split_block (e_if_taken->dest, stmt_end);
>> > +  bb_before_if = e_if_taken->src;
>> > +  bb_after_if = e_then_join->dest;
>
> On mainline when we do profile estimation before profiling instrumentaiton, now,
> you really want to update profile for performance here.
I am not sure I understand this.

>> > +  make_edge (bb_before_if, bb_after_if, EDGE_FALSE_VALUE);
>> > +}
>> > +
>> > +/* Transform:
>> > +
>> > +   ORIGINAL CODE
>> > +
>> > +   Into:
>> > +
>> > +   __gcov_sample_counter++;
>> > +   if (__gcov_sample_counter >= __gcov_sampling_rate)
>> > +     {
>> > +       __gcov_sample_counter = 0;
>> > +       ORIGINAL CODE
>> > +     }
>
> Hmm, I think the probability that internal loop of program will interfere with
> sampling rate is relatively high, but I see it is bit hard to do something about
> it.  Can we think of some very basic randomization of sampling_rate?

The predominant use case we have for this technique for server
workloads is as follows: Have a high value for __gcov_sampling_rate
(that should probably be named __gcov_sampling_interval)  during
server start up so that it reduces the overhead as well as ensures
that the startup period doesn't pollute the rest of the counters.
After startup, change the sampling interval to a small value (in many
cases, to 1) using the external interface in libgcov.c. In this use
case, randomization doesn't make sense during startup (since we want
to skip profile collection) as well as the steady phase (since the
interval is 1 or a small number).  If you want to have randomization
support, do you have any suggestions for how to make it work with low
sampling intervals?


Thanks,
Easwaran

> Honza