LGTM, however we need to iterate on this to make it more general. As far as  
I
can see changes made through the API is not handled after profiling has  
started.


http://codereview.chromium.org/402100/diff/1001/2001
File src/log.cc (right):

http://codereview.chromium.org/402100/diff/1001/2001#newcode650
Line 650: // TODO(mnaganov): Don't use a bogus size.
Please file a bug and change this to TODO(bugnumber).

http://codereview.chromium.org/402100/diff/1001/2001#newcode1239
Line 1239: void VisitFTI(Object* o) {
Looks as if this has some assumptions to how the object model has been
build. It is OK for now, but it needs to be more general, perhaps with
information from the embedding application on how to name the callback
functions.

http://codereview.chromium.org/402100/diff/1001/2001#newcode1257
Line 1257: // Properties are triples: [property name, entry point,
attributes].
No mentioning of Chromium.

http://codereview.chromium.org/402100/diff/1001/2002
File src/log.h (right):

http://codereview.chromium.org/402100/diff/1001/2002#newcode117
Line 117: V(BOUND_TAG,                      "Bound",
"bo")       \
I don't see why this is called bound object. A more natural name would
be just callback.

http://codereview.chromium.org/402100/diff/1001/2002#newcode204
Line 204: // Emits a code event for a callback of a bound object.
"callback of a bound object" -> "callback function"

http://codereview.chromium.org/402100/diff/1001/2002#newcode205
Line 205: static void BoundObjectCallbackEvent(const char* class_name,
Just CallbackEvent or CallbackFunctionEvent.

http://codereview.chromium.org/402100/diff/1001/2002#newcode275
Line 275: // Used for logging Chromium bindings callback entry points
Please don't mention Chromium in the V8 code. This is really not
Chromium specific.

http://codereview.chromium.org/402100/diff/1001/2002#newcode278
Line 278: static void LogChromiumBindingsCallbacks();
LogChromiumBindingsCallbacks -> LogCallbackFunctions

http://codereview.chromium.org/402100

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

Reply via email to