On 2015/04/24 12:08:13, marja (ooo - no reviews) wrote:
lgtm, much clearer now!
Pls run perf tryjobs before landing.
https://codereview.chromium.org/1102523003/diff/40001/src/preparser.cc
File src/preparser.cc (right):
https://codereview.chromium.org/1102523003/diff/40001/src/preparser.cc#newcode211
src/preparser.cc:211: int count_lines = 0;
Nit: Not really lines, but statements
Done.
https://codereview.chromium.org/1102523003/diff/40001/src/preparser.cc#newcode258
src/preparser.cc:258: // Our current definition of 'long and trivial) is:
Nit: ) -> '
Done.
https://codereview.chromium.org/1102523003/diff/40001/src/preparser.cc#newcode261
src/preparser.cc:261: if (maybe_reset && (!starts_with_identifier ||
++count_lines > 200)) {
IMO clearer:
if (!starts_with_identifier) maybe_reset = false;
if (maybe_reset && ++count_lines > 200) {
bookmark->Reset();
return;
}
Hmm. This was a bit of a micro-optimization, in that I expect this logic to
be
'approximately never' to be executed, and so I wanted to have it all inside
a
single if to keep its runtime impact minimal. Admittedly, this is a
micro-optimization that I'm not entirely sure about its worthwhile.
https://codereview.chromium.org/1102523003/diff/40001/src/scanner.h
File src/scanner.h (right):
https://codereview.chromium.org/1102523003/diff/40001/src/scanner.h#newcode769
src/scanner.h:769: uc32 bookmark_c0_;
Pls add a comment here about why we need to store the literals (at the
point
where we set a bookmark, scanning has already advanced, and we need to
recover
the literals, etc.)
Done.
https://codereview.chromium.org/1102523003/
--
--
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 and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
