Thanks!
http://codereview.chromium.org/6551011/diff/1/src/compiler.h File src/compiler.h (right): http://codereview.chromium.org/6551011/diff/1/src/compiler.h#newcode269 src/compiler.h:269: Handle<SharedFunctionInfo> shared); On 2011/02/22 15:11:19, Vitaly wrote:
Can we use info->shared_info() instead of passing another shared
function info?
If not, please document here why.
For the case when CompilationInfo is built up using Script object, shared_info isn't available from it. See Compiler::BuildFunctionInfo for an example. I've documented it. http://codereview.chromium.org/6551011/diff/1/src/cpu-profiler.cc File src/cpu-profiler.cc (right): http://codereview.chromium.org/6551011/diff/1/src/cpu-profiler.cc#newcode358 src/cpu-profiler.cc:358: SharedFunctionInfo *shared, On 2011/02/22 15:11:19, Vitaly wrote:
nit: Move *.
Done. http://codereview.chromium.org/6551011/diff/1/src/cpu-profiler.cc#newcode373 src/cpu-profiler.cc:373: SharedFunctionInfo *shared, On 2011/02/22 15:11:19, Vitaly wrote:
Same nit.
Done. http://codereview.chromium.org/6551011/diff/1/src/handles.cc File src/handles.cc (right): http://codereview.chromium.org/6551011/diff/1/src/handles.cc#newcode869 src/handles.cc:869: bool result = CompileLazyHelper(&info, KEEP_EXCEPTION); On 2011/02/22 15:11:19, Vitaly wrote:
Temporary "result" variable is now unnecessary.
Done. http://codereview.chromium.org/6551011/diff/1/src/log.cc File src/log.cc (right): http://codereview.chromium.org/6551011/diff/1/src/log.cc#newcode164 src/log.cc:164: sample->function = Memory::Address_at(sample->sp); On 2011/02/22 15:11:19, Vitaly wrote:
We should rename the "function" field in TickSample. How about
"tos_value"? Renamed it to "tos" -- it's shorter. http://codereview.chromium.org/6551011/diff/1/src/log.cc#newcode762 src/log.cc:762: if (code == Builtins::builtin(Builtins::LazyCompile)) return; On 2011/02/22 15:11:19, Vitaly wrote:
Can we make the caller do this check for us? (I think it should only
matter for
calls made from compiler.cc.)
Good idea. BTW, I forgot to add a similar check to cpu-profiler, so your suggestion eliminates an issue ;) LazyCompile builtins can only be passed from Compiler::RecordFunctionCompilation and Logger::LogCompiledFunctions, so I moved checks there. http://codereview.chromium.org/6551011/diff/1/src/log.cc#newcode785 src/log.cc:785: SharedFunctionInfo *shared, On 2011/02/22 15:11:19, Vitaly wrote:
Same nit.
Done. http://codereview.chromium.org/6551011/diff/1/src/log.h File src/log.h (right): http://codereview.chromium.org/6551011/diff/1/src/log.h#newcode210 src/log.h:210: SharedFunctionInfo *shared, On 2011/02/22 15:11:19, Vitaly wrote:
nit: Move *.
Done. http://codereview.chromium.org/6551011/diff/1/src/log.h#newcode214 src/log.h:214: SharedFunctionInfo *shared, On 2011/02/22 15:11:19, Vitaly wrote:
Same nit.
Done. http://codereview.chromium.org/6551011/diff/1/src/profile-generator.h File src/profile-generator.h (right): http://codereview.chromium.org/6551011/diff/1/src/profile-generator.h#newcode273 src/profile-generator.h:273: static const CodeEntry* kSfiCodeEntry; On 2011/02/22 15:11:19, Vitaly wrote:
Add one more "const" to make it really constant.
Ah, yes, stupid syntax. I've changed it to "static CodeEntry* const", because I really don't care about destination constness. http://codereview.chromium.org/6551011/diff/1/tools/ll_prof.py File tools/ll_prof.py (right): http://codereview.chromium.org/6551011/diff/1/tools/ll_prof.py#newcode403 tools/ll_prof.py:403: if match.group(6): On 2011/02/22 15:11:19, Vitaly wrote:
Could you please assign these groups (1, 4, 6) to locals (like it's
done for
"start_address") to make this code readable?
Done. http://codereview.chromium.org/6551011/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
