Thanks for the detailed change description! I'll need another quick pass.


-- Vitaly


http://codereview.chromium.org/2840018/diff/1/17
File src/builtins.cc (right):

http://codereview.chromium.org/2840018/diff/1/17#newcode1415
src/builtins.cc:1415: struct BuiltinDesc {
Can lines 1415--1453 be moved out of this function and the structures
used turned into real static data? If yes, it should help with stack
usage.

http://codereview.chromium.org/2840018/diff/1/18
File src/builtins.h (right):

http://codereview.chromium.org/2840018/diff/1/18#newcode179
src/builtins.h:179: // VM initialization.
s/VM/isolate ?

http://codereview.chromium.org/2840018/diff/1/18#newcode233
src/builtins.h:233: bool initialized() { return initialized_; }
"is_initialized" + "const".

http://codereview.chromium.org/2840018/diff/1/39
File src/isolate.h (right):

http://codereview.chromium.org/2840018/diff/1/39#newcode210
src/isolate.h:210: V(FunctionInfoListener*,
active_function_info_listener, NULL)                \
Document that this is a part of liveedit. (Right?)

http://codereview.chromium.org/2840018/diff/1/39#newcode551
src/isolate.h:551: int inline_runtime_lut_size() { return
inline_runtime_lut_size_; }
"const"

http://codereview.chromium.org/2840018/diff/1/39#newcode666
src/isolate.h:666: InlineRuntimeLUT* inline_runtime_lut_;
Please move the two LUT fields into a new InlineRuntimeFunctionsTable
class. It will improve readability a lot. In that class the entry array
can be inlined (as opposed to explicitly allocating it in the heap).
An entry type could be referenced externally by
InlineRuntimeFunctionsTable::Entry.

http://codereview.chromium.org/2840018/diff/1/41
File src/liveedit.h (right):

http://codereview.chromium.org/2840018/diff/1/41#newcode74
src/liveedit.h:74: static bool IsActive(Isolate* isolate);
Why isn't it using the isolate field?

http://codereview.chromium.org/2840018/diff/1/47
File src/runtime.h (right):

http://codereview.chromium.org/2840018/diff/1/47#newcode545
src/runtime.h:545: // buffers reused by BoyerMoore
nit: "Buffers" and add full stop at the end.

http://codereview.chromium.org/2840018/diff/1/50
File src/vm-state-inl.h (right):

http://codereview.chromium.org/2840018/diff/1/50#newcode78
src/vm-state-inl.h:78: Isolate* isolate = Isolate::Current();
I think we should save the pointer to the current isolate here and use
it in the destructor (guarded by an assert, of course). This should
address the todo above.

http://codereview.chromium.org/2840018/diff/1/51
File src/vm-state.cc (right):

http://codereview.chromium.org/2840018/diff/1/51#newcode1
src/vm-state.cc:1: // Copyright 2010 the V8 project authors. All rights
reserved.
We can delete this file now.

http://codereview.chromium.org/2840018/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to