Many comments about comments, but otherwise LGTM. -Ivan
http://codereview.chromium.org/42020/diff/1/2 File include/v8.h (right): http://codereview.chromium.org/42020/diff/1/2#newcode1916 Line 1916: static void SetAddHistogramSampleFunction(AddHistogramSampleCallback); Please add comments documenting these two new V8 API calls. http://codereview.chromium.org/42020/diff/1/6 File src/counters.cc (right): http://codereview.chromium.org/42020/diff/1/6#newcode68 Line 68: if (histogram_ != NULL) { I can guess why you would want to not call GetHistogram() at this point, but it would be clearer if you added a comment. http://codereview.chromium.org/42020/diff/1/6#newcode76 Line 76: These two functions could conceivably also be part of the header file. It is not clear what make GetHistogram() more special compared to Start() or Stop(). http://codereview.chromium.org/42020/diff/1/7 File src/counters.h (right): http://codereview.chromium.org/42020/diff/1/7#newcode49 Line 49: static void SetAddHistogramSampleFunction(AddHistogramSampleCallback f) { Small comment for the two new functions or extend the comment above to be more generic. http://codereview.chromium.org/42020/diff/1/7#newcode68 Line 68: static void *CreateHistogram(const char* name, Comment missing similar to the one above, describing in what circumstances this gets called. http://codereview.chromium.org/42020/diff/1/7#newcode76 Line 76: static void AddHistogramSample(void* histogram, int sample) { ditto http://codereview.chromium.org/42020/diff/1/7#newcode179 Line 179: // HistogramTimer t = { L"t:foo", NULL, false }; Histograms are not created with a "t:" prefix as far as I can tell. http://codereview.chromium.org/42020/diff/1/7#newcode196 Line 196: return histogram_ != NULL && start_time_ != 0 && stop_time_ == 0; Usually we try to not rely on C precedence rules and format conditions in this way: (a != b) && (b == c) http://codereview.chromium.org/42020 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
