https://codereview.chromium.org/17642009/diff/4001/include/v8-profiler.h
File include/v8-profiler.h (right):

https://codereview.chromium.org/17642009/diff/4001/include/v8-profiler.h#newcode79
include/v8-profiler.h:79: int GetScriptId() const;
In v8.h Script::Id returns v8::Local<v8::Value> and internally it seems
to be stored as i::String:
https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/objects-inl.h&sq=package:chromium&type=cs&l=4437&rcl=1372117930.
I think we should be consistent with v8::Script::Id() type here and use
Local<Value> as the return type.

Or if it is always int internally we could deprecate
v8::Local<v8::Value> v8::Script::Id() and introduce int
v8::Script::GetId() instead.

https://codereview.chromium.org/17642009/diff/4001/src/cpu-profiler.cc
File src/cpu-profiler.cc (right):

https://codereview.chromium.org/17642009/diff/4001/src/cpu-profiler.cc#newcode96
src/cpu-profiler.cc:96: }
In case when there are no |info| you may still be able to retrieve
Script from SharedFunctionInfo (here it is already passed as Address but
the signature can be changed to accept SharedFunctionInfo* instead).

https://codereview.chromium.org/17642009/diff/4001/src/profile-generator.cc
File src/profile-generator.cc (right):

https://codereview.chromium.org/17642009/diff/4001/src/profile-generator.cc#newcode275
src/profile-generator.cc:275: entry_->script_id());
I'd move this before the node id in the output.

https://codereview.chromium.org/17642009/diff/4001/test/cctest/test-cpu-profiler.cc
File test/cctest/test-cpu-profiler.cc (right):

https://codereview.chromium.org/17642009/diff/4001/test/cctest/test-cpu-profiler.cc#newcode473
test/cctest/test-cpu-profiler.cc:473: if (function_name.length() &&
(*function_name)[0] != '(')
Missing {}

https://codereview.chromium.org/17642009/diff/4001/test/cctest/test-cpu-profiler.cc#newcode580
test/cctest/test-cpu-profiler.cc:580:
CheckScriptIds(script->Id()->ToInt32()->Value(), root);
Would be nice to have additional test for a profile where nodes have
different script ids.

https://codereview.chromium.org/17642009/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to