LGTM for files other than plot-timer-events.js. I'd like to have some V8
expert
(who has reviewed plot-timer-events.js) take a look at plot-timer-events.js.
https://codereview.chromium.org/12040075/diff/5001/tools/plot-timer-events.js
File tools/plot-timer-events.js (right):
https://codereview.chromium.org/12040075/diff/5001/tools/plot-timer-events.js#newcode85
tools/plot-timer-events.js:85: return this[this.length-1];
Nit: this.length - 1 (Spaces needed around '-')
https://codereview.chromium.org/12040075/diff/5001/tools/plot-timer-events.js#newcode132
tools/plot-timer-events.js:132: function
AccountForDistortionToMs(timestamp) {
I don't fully understand what 'AccountForDistortionToMs' means. Maybe
'AlignTimestampToMillisecondResolution' ?
https://codereview.chromium.org/12040075/diff/5001/tools/plot-timer-events.js#newcode174
tools/plot-timer-events.js:174: // Find out about the thread_id.
Nit: Find out the thread_id.
https://codereview.chromium.org/12040075/diff/5001/tools/plot-timer-events.js#newcode175
tools/plot-timer-events.js:175: var finished_event = TimerEvents[name];
if (finished_event === undefined) return;
https://codereview.chromium.org/12040075/diff/5001/tools/plot-timer-events.js#newcode181
tools/plot-timer-events.js:181: assert(finished_event ===
event_stack[thread_id].pop());
I want to make the error message more verbose e.g. what event is not
balanced in the log. You can pass a debug message to assert().
https://codereview.chromium.org/12040075/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group, send email to
[email protected].
For more options, visit https://groups.google.com/groups/opt_out.
