LGTM
http://codereview.chromium.org/149195/diff/1/7 File test/mjsunit/tools/tickprocessor.js (right): http://codereview.chromium.org/149195/diff/1/7#newcode29 Line 29: // Files: tools/splaytree.js tools/codemap.js tools/csvparser.js tools/consarray.js tools/profile.js tools/profile_view.js tools/logreader.js tools/tickprocessor.js Over-long line. http://codereview.chromium.org/149195/diff/1/10 File tools/tickprocessor.js (right): http://codereview.chromium.org/149195/diff/1/10#newcode520 Line 520: 'Show only ticks from JS VM state'], It would be better if this was an object with keys instead of an array. Then you could use dispatch.key, dispatch.default and dispatch.helpText instead of dispatch[0], dispatch[1] etc. below. You can defer this change to another changelist if you like. http://codereview.chromium.org/149195/diff/1/10#newcode596 Line 596: s = s + (new Array(len - s.length + 1).join(' ')); I hope this part isn't performance critical in any way! Couldn't you just keep a long string of spaces around and use substring to chop off as many as you need to pad. http://codereview.chromium.org/149195 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
