Resending in plain text mode...sigh. Teresa
On Thu, Jul 26, 2012 at 1:32 PM, Teresa Johnson <tejohn...@google.com> wrote: > > > > > On Thu, Jul 26, 2012 at 11:26 AM, <davi...@google.com> wrote: >> >> >> http://codereview.appspot.com/6351086/diff/1/gcc/gcov-io.h >> File gcc/gcov-io.h (right): >> >> http://codereview.appspot.com/6351086/diff/1/gcc/gcov-io.h#newcode396 >> gcc/gcov-io.h:396: gcov_unsigned_t num_hot_counters;/* number of >> >> counters to reach a given >> Should it be made into an array accessed with pre-defined indices? > > > You mean provide an array of values for different thresholds? In that case > both num_hot_counters and hot_cutoff_value would need to be arrays (oh, I see > now that you indicate that below too). And we would possibly need another > array to hold the thresholds, unless they are hard coded values. I've been > thinking about extending this to provide data for more threshold values, but > was holding that off for a follow-on, when I had a better idea of how the > expanded summary info would be used. > > Honza - you were talking about using this type of summary info to guide > hot/cold decisions, do you find the current single threshold data useful > enough? > >> >> http://codereview.appspot.com/6351086/diff/1/gcc/gcov-io.h#newcode399 >> gcc/gcov-io.h:399: num_hot_counters. */ >> Similarly for this one -- or put the above two values into a pair and >> declare an array of the pairs? >> >> http://codereview.appspot.com/6351086/diff/1/libgcc/libgcov.c >> File libgcc/libgcov.c (right): >> >> http://codereview.appspot.com/6351086/diff/1/libgcc/libgcov.c#newcode332 >> libgcc/libgcov.c:332: if (cs_ptr->sum_all*cutoff_perc < cs_ptr->sum_all) >> space needed >> > > I think I have added the space you wanted (around the '*' char) - let me know > if you meant something else. > >> >> http://codereview.appspot.com/6351086/diff/1/libgcc/libgcov.c#newcode332 >> libgcc/libgcov.c:332: if (cs_ptr->sum_all*cutoff_perc < cs_ptr->sum_all) >> space needed. Is it better to check if (cs_ptr->sum_all * cutoff_perc >> /cutoff_perc != cs_ptr->sum_all)? > > > Good suggestion - changed. > >> >> >> http://codereview.appspot.com/6351086/diff/1/libgcc/libgcov.c#newcode718 >> libgcc/libgcov.c:718: if (cs_tprg->num_hot_counters > >> cs_prg->num_hot_counters) >> Why guard the update under !cs_prg->runs++ --> the condition is used to >> identify if it is a first write or a merged write. (verify with a test >> case to see if the merge of the summary is correct). > > > In fact the ">" check doesn't make sense if this is the first write, which is > currently how I have it guarded. The question is how to best "merge" this > information. After thinking about this, I am going to move this code out from > under the !cs_prg->runs++ check, so that on a merge, we use the summary with > the largest num_hot_counters. > > I am going to do a little testing with my changes and will upload a new patch. > > Thanks, > Teresa > >> >> >> http://codereview.appspot.com/6351086/ > > > > > -- > Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413 > -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
