On Fri, Apr 29, 2011 at 9:12 AM, Silvius Rus <silvius....@gmail.com> wrote:
>> How is code-size affected with this patch, non-instrumented vs.
>> regular-instrumented vs. sample-instrumented?
>
> I don't have the numbers, but the increase in code size from
> regular-instrumented to sample-instrumented is larger than that from
> non-instrumented to sample-instrumented. Easwaran, could you please
> build a few binaries at -O2, -O2 -fprofile-generate and -O2
> -fprofile-generate -fprofile-generate-sampling, and send out the text
> size ratios considering -O2 as the baseline?
>
>> > @@ -292,6 +474,15 @@ gimple_gen_edge_profiler (int edgeno, edge e)
>> > gimple_assign_lhs (stmt1), one);
>> > gimple_assign_set_lhs (stmt2, make_ssa_name (gcov_type_tmp_var, stmt2));
>> > stmt3 = gimple_build_assign (unshare_expr (ref), gimple_assign_lhs
>> > (stmt2));
>> > +
>> > + if (flag_profile_generate_sampling)
>> > + {
>> > + slot = htab_find_slot_with_hash (instrumentation_to_be_sampled,
>> > stmt1,
>> > + htab_hash_pointer (stmt1), INSERT);
>> > + gcc_assert (!*slot);
>> > + *slot = stmt1;
>> > + }
>> > +
>>
>> What's the reason to delay the sampling transform and not apply it here?
>> At least a comment should be added for this.
>
> Sorry, I just don't remember.
Is this because altering the CFG while iterating over the CFG edges in
instrument_edges (profile.c) is not safe?
-Easwaran
>> > +DEFPARAM (PARAM_PROFILE_GENERATE_SAMPLING_RATE,
>> > + "profile-generate-sampling-rate",
>> > + "sampling rate with -fprofile-generate-sampling",
>> > + 100, 0, 2000000000)
>>
>> Zero is really ok?
>
> It wouldn't break anything, but a rate of 0 doesn't make sense. It
> should be 1. And the default should be 101 instead of 100 (and
> generally a prime).
>
> While we're at it, let me bring up an important practical FDO issue
> related to this. As David mentioned, besides reducing overhead we
> have used this to implement selective collection. This essentially
> requires libgcov to provide a basic public interface:
> reset()
> start()
> stop()
> save()
> I implemented them by playing with the sampling rate, but a clearer
> and supported interface would be useful. Also, there was one annoying
> detail that I could not figure out. When you build with
> -fprofile-generate=./fdoprof, the output gets dumped under
> ./fdoprof/..., but there seems to be no easy way to know this path
> within the profiling process. For Google servers this makes
> collection fragile. The user generally does not have access to the
> server file system. I implemented a collection mechanism so the user
> can tell the server "copy the profiles from ./fdoprof to some shared
> location". But now you need to pass the exact path when building
> *and* collecting profiles, which may get separated in time, thus the
> process is fragile. It would be good to keep a copy of the path
> prefix and have it accessible through a public interface as well.
> Then once an instrumented binary is built, it will know the root of
> the profile output directory and thus relieve the user from the
> responsibility of knowing it.
>
> Silvius
>
