+ 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