New patch set uploaded that addresses comments.
Moving the profiler_ticks field to SharedFunctionInfo might affect
performance;
proper testing is currently blocked on an unrelated bug that is triggered
when
running earley-boyer.
http://codereview.chromium.org/8700008/diff/9001/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):
http://codereview.chromium.org/8700008/diff/9001/src/ia32/full-codegen-ia32.cc#newcode135
src/ia32/full-codegen-ia32.cc:135: if (FLAG_crankshaft &&
On 2011/12/13 11:53:01, fschneider wrote:
If a function has optimization disabled via DisableOptimization()
because we
can't optimize it or we deoptimized frequently, we should not add code
for
counting.
Done.
http://codereview.chromium.org/8700008/diff/9001/src/ia32/full-codegen-ia32.cc#newcode136
src/ia32/full-codegen-ia32.cc:136: info->function()->is_primitive() &&
On 2011/12/12 11:31:21, fschneider wrote:
What is a primitive function? I can't find the source code for this
predicate.
Basically a function that doesn't contain loops, calls, or
un-inline-able statements. The definition is in
http://codereview.chromium.org/8677008/
http://codereview.chromium.org/8700008/diff/9001/src/objects.h
File src/objects.h (right):
http://codereview.chromium.org/8700008/diff/9001/src/objects.h#newcode5464
src/objects.h:5464: inline int profiler_ticks();
On 2011/12/13 11:53:01, fschneider wrote:
On 2011/12/12 11:31:21, fschneider wrote:
> Shouldn't be this on SharedFunctionInfo instead?
This increases the size of each JSFunction by 1 word from 36 to 40
(11%). This
could hurt memory-constrained devices on programs with lots of
closures. I'd
prefer a solution without additional space cost.
Done.
http://codereview.chromium.org/8700008/diff/9001/src/runtime-profiler.cc
File src/runtime-profiler.cc (left):
http://codereview.chromium.org/8700008/diff/9001/src/runtime-profiler.cc#oldcode59
src/runtime-profiler.cc:59: static const int kSizeLimit = 1500;
On 2011/12/12 11:31:21, fschneider wrote:
The reason for looking at the size in some form was that large
functions are
more expensive to optimize, so we want to be sure they're worth
optimizing by
requiring more profiler ticks. The metric used is maybe not ideal, but
some form
of taking size into account would be still useful. It seemed to help
on ia32.
I see the reasoning and agree in principle, but with the new approach I
haven't been able to measure any benefit to considering the function
size.
http://codereview.chromium.org/8700008/diff/9001/src/runtime-profiler.cc
File src/runtime-profiler.cc (right):
http://codereview.chromium.org/8700008/diff/9001/src/runtime-profiler.cc#newcode82
src/runtime-profiler.cc:82: PrintF(" for recompilation, reason: %d",
reason);
On 2011/12/13 11:53:01, fschneider wrote:
Print reason in English instead of just a number.
Done.
http://codereview.chromium.org/8700008/diff/9001/src/runtime-profiler.cc#newcode163
src/runtime-profiler.cc:163: Optimize(function, 1);
On 2011/12/13 11:53:01, fschneider wrote:
Name instead of an integer constant?
Done.
http://codereview.chromium.org/8700008/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
