+ if (output_to_real_object_first) {
+ if (copy_file(tmp_obj, cached_obj, enable_compression) != 0) {
+ cc_log("Failed to move %s to %s: %s", tmp_obj, cached_obj,
strerror(errno));
+ stats_update(STATS_ERROR);
+ failed();
+ }
Maybe you should hardlink, if CCACHE_HARDLINK is set. Unless
copy_file already does that?
+ if (profile_use) {
+ /* Calculate gcda name */
+ char *gcda_name;
+ char *base_name;
+ output_to_real_object_first = true;
+ base_name = remove_extension(output_obj);
+ gcda_name = format("%s/%s.gcda", profile_dir, base_name);
+ cc_log("Adding profile data %s to our hash", gcda_name);
+ /* Add the gcda to our hash */
+ hash_delimiter(hash, "-fprofile-use");
+ hash_file(hash, gcda_name);
Could you add a comment saying that profile_dir is the cwd if
-fprofile-use doesn't have a parameter? I initially thought it was
null (and that output_obj was an absolute path), and was confused as
to how this might work.
What happens if I specify -fprofile-use=dir1
-fbranch-probabilities=dir2? I think ccache should bail and say it's
too hard in this and similar cases.
This looks good to me, for whatever that's worth. :)
-Justin
On Mon, Jul 25, 2011 at 3:19 PM, Chris AtLee <ch...@atlee.ca> wrote:
> I've pushed some changes up here that I hope addresses all the comments:
> https://github.com/catlee/ccache/compare/jrosdahl:master...catlee:profile
>
>
> On Fri, Jul 22, 2011 at 3:19 PM, Chris AtLee <ch...@atlee.ca> wrote:
>> On Fri, Jul 22, 2011 at 1:49 PM, Joel Rosdahl <j...@rosdahl.net> wrote:
>>> Justin Lebar's point that the cwd probably needs to be hashed seems
>>> valid. Other than that, I think it looks generally fine, but I only have
>>> limited knowledge about the -fprofile-* options so I can't say I
>>> understand their interaction completely. Some comments and questions,
>>> though:
>>>
>>> I think that the "output_to_real_object_first mode" should really be the
>>> default and only mode. That would of course mean that the things in
>>> from_cache() that need to be done after storing an object in the cache
>>> should be refactored out somehow, but we can do that later.
>>
>> Sounds good. I'll need to add another flag to indicate that we need to
>> hash cwd then if output_to_real_object_first is the default.
>>
>>> What about the -fprofile-generate=path and -fprofile-use=path forms?
>>> Should those be detected as well?
>>
>> For -fprofile-generate, I don't think we need to worry about the
>> profile directory since the inputs to the compilation aren't affected
>> by where the profile data is stored. -fprofile-use does need to know
>> where to look so that it can add the profile data to the hash. I'm
>> half tempted to remove -fprofile-use support, I'm not sure how likely
>> you are to get cache hits there. Running the same executable twice in
>> a row generates different profile data.
>>
>>> tmp_obj is freed at the end of to_cache(), so output_obj will refer to
>>> freed memory when dereferenced in from_cache() later. An x_strdup is needed.
>>
>> Fixed.
>>
>>> //-style comments won't compile on systems with C89 compilers, so use
>>> /**/ instead.
>>
>> Fixed.
>>
>>> You should probably use hash_delimiter(hash, "-fprofile-use") or so
>>> before hashing the gcda file (and then I guess that the hashing of "no
>>> data" won't be necessary).
>>
>> Ah, that's what that function is for!
>>
>> Thanks for the feedback, I should have some new commits up shortly.
>>
>> Cheers,
>> Chris
> _______________________________________________
> ccache mailing list
> ccache@lists.samba.org
> https://lists.samba.org/mailman/listinfo/ccache
>
_______________________________________________
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache
