LG except for the way StackGuard sets limits on Heap.
-- Vitaly http://codereview.chromium.org/2715004/diff/36002/42005 File src/debug.h (right): http://codereview.chromium.org/2715004/diff/36002/42005#newcode783 src/debug.h:783: Isolate* isolate = Isolate::Current(); Add TODO(isolates) to check this is the same isolate as in the constructor. http://codereview.chromium.org/2715004/diff/36002/42006 File src/execution.cc (right): http://codereview.chromium.org/2715004/diff/36002/42006#newcode46 src/execution.cc:46: // ClearThreadLocal(&thread_local_) is called by Isolate once it is safe. Looks really unfortunate. I think it should be safe provided Heap is initialized before StackGuard. http://codereview.chromium.org/2715004/diff/36002/42006#newcode336 src/execution.cc:336: ClearThreadLocal(&blank); Can we decouple clearing the thread local from setting limits in the isolate? The current situation looks too complicated and the comment describing its previous state doesn't really help. http://codereview.chromium.org/2715004/diff/36002/42007 File src/execution.h (right): http://codereview.chromium.org/2715004/diff/36002/42007#newcode204 src/execution.h:204: // TODO(isolates): Technically this could be calculated directly from Move this next to other fields of this class. http://codereview.chromium.org/2715004/diff/36002/42008 File src/heap.cc (right): http://codereview.chromium.org/2715004/diff/36002/42008#newcode3947 src/heap.cc:3947: ASSERT(isolate_ != NULL); Also check it's the current one? http://codereview.chromium.org/2715004/diff/36002/42012 File src/isolate.h (right): http://codereview.chromium.org/2715004/diff/36002/42012#newcode66 src/isolate.h:66: static bool SetDefaultThreadResourceConstraints(ResourceConstraints* rc); Never defined? http://codereview.chromium.org/2715004/diff/36002/42012#newcode89 src/isolate.h:89: StackGuard* stack_guard() { nit: Fit on one line. http://codereview.chromium.org/2715004/diff/36002/42012#newcode155 src/isolate.h:155: void StackGuard::set_interrupt_limits(const ExecutionAccess& lock) { I think this function and the next one are okay to move to execution.cc (where we can freely include the isolate header). http://codereview.chromium.org/2715004/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
