https://chromiumcodereview.appspot.com/9976003/diff/1/src/frames.cc File src/frames.cc (right):
https://chromiumcodereview.appspot.com/9976003/diff/1/src/frames.cc#newcode1372 src/frames.cc:1372: void Init() { On 2012/04/04 20:47:57, danno wrote:
How about putting this directly in SetUpJSCallerSavedCodeData? I don't
see the
need for the function on the struct any more.
Done. https://chromiumcodereview.appspot.com/9976003/diff/1/src/platform-macos.cc File src/platform-macos.cc (right): https://chromiumcodereview.appspot.com/9976003/diff/1/src/platform-macos.cc#newcode881 src/platform-macos.cc:881: } On 2012/04/04 20:47:57, danno wrote:
Can you please move these back to where they use to be in the file? It
makes the
diff shorter and the change more compact. Also applies to other
platform files. I had unfortunately to move this function here to make it be declared after the SamplerThread class which it uses (SamplerThread::SetUp()). We have to move either OS::SetUp() or SamplerThread. https://chromiumcodereview.appspot.com/9976003/diff/1/src/v8.cc File src/v8.cc (right): https://chromiumcodereview.appspot.com/9976003/diff/1/src/v8.cc#newcode279 src/v8.cc:279: SamplerRegistry::GlobalSetUp(); On 2012/04/04 20:47:57, danno wrote:
nit: Perhaps just SetUp rather than GlobalSetUp, since all of the
other methods
of the same kind use that convention.
Done. https://chromiumcodereview.appspot.com/9976003/diff/1/src/v8.cc#newcode280 src/v8.cc:280: ExternalReference::GlobalSetUp(); On 2012/04/04 20:47:57, danno wrote:
Same here
Done. https://chromiumcodereview.appspot.com/9976003/diff/1/src/x64/disasm-x64.cc File src/x64/disasm-x64.cc (right): https://chromiumcodereview.appspot.com/9976003/diff/1/src/x64/disasm-x64.cc#newcode348 src/x64/disasm-x64.cc:348: const InstructionTable& instruction_table_; On 2012/04/04 20:47:57, danno wrote:
We don't use const references in V8. Please use const *.
Done. https://chromiumcodereview.appspot.com/9976003/diff/14003/src/v8.cc File src/v8.cc (right): https://chromiumcodereview.appspot.com/9976003/diff/14003/src/v8.cc#newcode267 src/v8.cc:267: RuntimeProfiler::GlobalSetUp(); For this one, we have to keep it as GlobalSetUp() since RuntimeProfiler already contains a non-static SetUp() method. I have just made it "GlobalSetUp" instead of "GlobalSetup" for consistency with the other ones. https://chromiumcodereview.appspot.com/9976003/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
