LGTM. Thanks for doing the refactoring work.
-- Vitaly http://codereview.chromium.org/2715004/diff/38002/40007 File src/execution.cc (right): http://codereview.chromium.org/2715004/diff/38002/40007#newcode350 src/execution.cc:350: ThreadLocal blank; Can we make its constructor call Clear()? http://codereview.chromium.org/2715004/diff/38002/40007#newcode426 src/execution.cc:426: if (thread_local_.Initialize()) { isolate_->heap()->SetStackLimits(); } nit: No {} for one-line ifs. http://codereview.chromium.org/2715004/diff/38002/40007#newcode624 src/execution.cc:624: // Perform preemption. Unrelated to your change, but this looks like a race when debugger support is not enabled. We read the undefined value before unlocker is destructed. Please put it in a {} block. http://codereview.chromium.org/2715004/diff/38002/40008 File src/execution.h (right): http://codereview.chromium.org/2715004/diff/38002/40008#newcode204 src/execution.h:204: Isolate* isolate_; // TODO(isolates): Technically this could be calculated The comment was formatted correctly. I meant that the field itself (and its comment) should be moved to be next to other fields of this class (to line 270). http://codereview.chromium.org/2715004/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
