Expose optimization info via runtime functions

Expose optimization info via runtime functions


TEST=mjsunit/assert-opt-and-deopt.js

Committed: http://code.google.com/p/v8/source/detail?r=7813

[Jakob Kummerow, 2011-04-21 10:53:42]

Does this satisfy your requirements? I'm happy to implement another approach if
you feel there's a need.

[danno, 2011-04-28 17:27:51]

LGTM.

[Jakob Kummerow, 2011-05-03 08:13:25]

I had to extend this quite a bit to make it work with the --stress-opt,
--always-opt and --nocrankshaft flags. Please take another look.

[Jakob Kummerow, 2011-05-04 07:42:52]

Mike: please take a look at the .js file.

[danno, 2011-05-04 07:47:04]

Non-JS LGTM with suggested name change.

http://codereview.chromium.org/6879108/diff/2001/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/6879108/diff/2001/src/runtime.cc#newcode7427
src/runtime.cc:7427: RUNTIME_FUNCTION(MaybeObject*, Runtime_IsOptimizedFunction)
{
Perhaps this should be called FunctionOptimizationStatus

[Mike West, 2011-05-04 08:18:02]

LGTM with nits.

http://codereview.chromium.org/6879108/diff/2001/test/mjsunit/assert-opt-and-...
File test/mjsunit/assert-opt-and-deopt.js (right):

http://codereview.chromium.org/6879108/diff/2001/test/mjsunit/assert-opt-and-...
test/mjsunit/assert-opt-and-deopt.js:38: this.NEVER = 4;
Nit: These can be pulled out into a static member of `OptTracker`, and would
probably be best as a separate enum.  See
http://codesearch.google.com/codesearch/p?vert=chromium#OAMlx_jo-ck/src/chrom...
for an example.

http://codereview.chromium.org/6879108/diff/2001/test/mjsunit/assert-opt-and-...
test/mjsunit/assert-opt-and-deopt.js:39: this.opt_counts_ = new Object();
Nit: Object literals (`{}`) are preferred.

http://codereview.chromium.org/6879108/diff/2001/test/mjsunit/assert-opt-and-...
test/mjsunit/assert-opt-and-deopt.js:52: return;
Nit: If asserts are disabled, you should probably return `true` here; otherwise
you're returning `undefined`, which evaluates to `false`.  `if
(tracker.AssertOptCount(x, y))` would fail...

http://codereview.chromium.org/6879108/diff/2001/test/mjsunit/assert-opt-and-...
test/mjsunit/assert-opt-and-deopt.js:77: if (raw_optimized == this.ALWAYS ||
Nit: Use `DisableAsserts_`.

http://codereview.chromium.org/6879108/diff/2001/test/mjsunit/assert-opt-and-...
test/mjsunit/assert-opt-and-deopt.js:155: for (var i = 0; i < 5; i++) f("a");
Nit: You're intentionally switching from numbers to strings here, and it's
almost certainly clear to V8 developers that the optimizations are
type-specific.  It wasn't clear to me, though... A comment would still be nice.
:)

[Jakob Kummerow, 2011-05-06 15:58:15]

Thanks for the comments; I finally got around to addressing them.
I'll land this on Monday morning.

http://codereview.chromium.org/6879108/diff/2001/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/6879108/diff/2001/src/runtime.cc#newcode7427
src/runtime.cc:7427: RUNTIME_FUNCTION(MaybeObject*, Runtime_IsOptimizedFunction)
{
On 2011/05/04 07:47:04, danno wrote:
> Perhaps this should be called FunctionOptimizationStatus

Done.

http://codereview.chromium.org/6879108/diff/2001/test/mjsunit/assert-opt-and-...
File test/mjsunit/assert-opt-and-deopt.js (right):

http://codereview.chromium.org/6879108/diff/2001/test/mjsunit/assert-opt-and-...
test/mjsunit/assert-opt-and-deopt.js:38: this.NEVER = 4;
On 2011/05/04 08:18:03, Mike West wrote:
> Nit: These can be pulled out into a static member of `OptTracker`, and would
> probably be best as a separate enum.  See
>
http://codesearch.google.com/codesearch/p?vert=chromium#OAMlx_jo-ck/src/chrom...
> for an example.

Done.

http://codereview.chromium.org/6879108/diff/2001/test/mjsunit/assert-opt-and-...
test/mjsunit/assert-opt-and-deopt.js:39: this.opt_counts_ = new Object();
On 2011/05/04 08:18:03, Mike West wrote:
> Nit: Object literals (`{}`) are preferred.

Done.

http://codereview.chromium.org/6879108/diff/2001/test/mjsunit/assert-opt-and-...
test/mjsunit/assert-opt-and-deopt.js:52: return;
On 2011/05/04 08:18:03, Mike West wrote:
> Nit: If asserts are disabled, you should probably return `true` here;
otherwise
> you're returning `undefined`, which evaluates to `false`.  `if
> (tracker.AssertOptCount(x, y))` would fail...

This function doesn't have a return value in any case. It's not supposed to be
used inside if() conditionals. The same is true for all OptTracker.Assert*
functions.

http://codereview.chromium.org/6879108/diff/2001/test/mjsunit/assert-opt-and-...
test/mjsunit/assert-opt-and-deopt.js:77: if (raw_optimized == this.ALWAYS ||
On 2011/05/04 08:18:03, Mike West wrote:
> Nit: Use `DisableAsserts_`.

Done.

http://codereview.chromium.org/6879108/diff/2001/test/mjsunit/assert-opt-and-...
test/mjsunit/assert-opt-and-deopt.js:155: for (var i = 0; i < 5; i++) f("a");
On 2011/05/04 08:18:03, Mike West wrote:
> Nit: You're intentionally switching from numbers to strings here, and it's
> almost certainly clear to V8 developers that the optimizations are
> type-specific.  It wasn't clear to me, though... A comment would still be
nice.
> :)

Done.