LGTM. Biggest concern is the atexit call from within V8.

http://codereview.chromium.org/39179/diff/1/8
File src/objects.h (right):

http://codereview.chromium.org/39179/diff/1/8#newcode2263
Line 2263: int ObjectSize() {
This name is probably too generic since it doesn't include the entire
object (reloc info, scope info, and padding does not count). It's not
really the entire size of the object.

http://codereview.chromium.org/39179/diff/1/9
File src/oprofile-agent.cc (right):

http://codereview.chromium.org/39179/diff/1/9#newcode44
Line 44: // Disable code moving by GC
Terminate comment with .

http://codereview.chromium.org/39179/diff/1/10
File src/oprofile-agent.h (right):

http://codereview.chromium.org/39179/diff/1/10#newcode60
Line 60: // Size of the buffer that is used for composing code areas
names
Terminate comment with .

http://codereview.chromium.org/39179/diff/1/11
File src/v8.cc (right):

http://codereview.chromium.org/39179/diff/1/11#newcode93
Line 93: atexit(V8::TearDown);
I'm not sure this is a good idea in the context of Chromium. Maybe you
should just make the shell sample and d8 do explicit tear down or use
the atexit from there? It really should be up to the embedder to invoke
V8::TearDown.

http://codereview.chromium.org/39179

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

Reply via email to