First set of comments. -Ivan
http://codereview.chromium.org/67100/diff/1/2 File src/d8.cc (right): http://codereview.chromium.org/67100/diff/1/2#newcode329 Line 329: Counter* counter = counter_map_->Lookup(name); I know it should not happen, but you should assert that the counter you found matches the histogram state that is being requested. http://codereview.chromium.org/67100/diff/1/2#newcode362 Line 362: Counter* counter = static_cast<Counter*>(histogram); Please use reinterpret_cast instead of static_cast here according to the C++ style guide. http://codereview.chromium.org/67100/diff/1/3 File src/d8.h (right): http://codereview.chromium.org/67100/diff/1/3#newcode53 Line 53: int32_t total_; Personally I find it confusing how these fields are used in the counter and histogram cases. This is probably because they conflict in their use. How about using something like this, you would need to adjust the names of the accessors and callers accordingly: int32_t count_; // Number of events or samples in the case of histogram use. int64_t value_; // Value is only used when the Counter is used as a histogram. It represents a total sum of all added samples. You can delay overflows for the value this way as we do not expose the pointer to this memory location to V8 internals directly. http://codereview.chromium.org/67100 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
