Thanks for comments!

Relaying some offline code review discussions: mstarzinger@ suggested adding a
flag to disable parallel parsing altogether, and I did that.


https://codereview.chromium.org/214883002/diff/530001/src/parser-thread.cc
File src/parser-thread.cc (right):

https://codereview.chromium.org/214883002/diff/530001/src/parser-thread.cc#newcode60
src/parser-thread.cc:60: mutex_.Lock();
On 2014/04/16 11:55:35, Sven Panne wrote:
Use LockGuard here...

Done.

https://codereview.chromium.org/214883002/diff/530001/src/parser-thread.cc#newcode70
src/parser-thread.cc:70: mutex_.Lock();
On 2014/04/16 11:55:35, Sven Panne wrote:
... and here.

Done.

https://codereview.chromium.org/214883002/diff/530001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/214883002/diff/530001/src/parser.cc#newcode898
src/parser.cc:898: new ExternalOneByteStringUtf16CharacterStream(
On 2014/04/16 11:55:35, Sven Panne wrote:
Extract the creation of the Utf16CharacterStream* out into a separate
function
(maybe returning NULL when it is not an ASCII/TwoByte string). If it
returns
non-NULL, we do exactly the same thing, something which is not very
obvious
right now and furthermore makes half of the comment above superfluous
(or at
least this part should be moved as well).

Extracted the Stream creation. But I don't get what you mean by "we do
exactly the same thing, something which is not very obvious right now
and furthermore makes half of the comment above superfluous (or  at
least this part should be moved as well)."

https://codereview.chromium.org/214883002/diff/530001/src/parser.cc#newcode953
src/parser.cc:953: if (thread_) {
On 2014/04/16 11:55:35, Sven Panne wrote:
Lift this block out into a separate method, it's used identically
below.

Done, and I also added a function for (possibly) starting the thread,
where the above thread-starting code goes.

https://codereview.chromium.org/214883002/diff/530001/src/parser.cc#newcode3542
src/parser.cc:3542: } else if (thread_) {
On 2014/04/16 11:55:35, Sven Panne wrote:
Hmmm, the block below contains some heavy copy-n-paste from the code
above, can
we somehow reduce this?

Done.

https://codereview.chromium.org/214883002/

--
--
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.

Reply via email to