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