Investigate & fix issues around usage_count and deoptimization_count

Investigate & fix issues around usage_count and deoptimization_count

BUG=
R=fschneider@google.com

Committed: https://github.com/dart-lang/sdk/commit/1595117898c5c40a7c3ba9c42028b9b0cd23a8a5

[srdjan, 2015-12-30 23:50:30]

srdjan@google.com changed reviewers:
+ fschneider@google.com

[Florian Schneider, 2016-01-04 11:31:38]

What was the issue fixed here?

https://codereview.chromium.org/1559653002/diff/40001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/1559653002/diff/40001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:471: // A function with huge unoptimized code canb become
non-optimizable
s/canb/can/

https://codereview.chromium.org/1559653002/diff/40001/runtime/vm/object.cc
File runtime/vm/object.cc (left):

https://codereview.chromium.org/1559653002/diff/40001/runtime/vm/object.cc#ol...
runtime/vm/object.cc:2815: a.DisableCode();
What about the other call-sites of DisableCode()? Also check HasCodes there?

https://codereview.chromium.org/1559653002/diff/40001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1559653002/diff/40001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:2816: if (a.HasCodes()) {
If you always check HasCodes before calling DisableCode, maybe add an ASSERT
inside DisableCode()

ASSERT(HasCodes())

and remove the check for a null code array there.

[srdjan, 2016-01-04 18:29:53]

Reason for this change:

Observed excessive optimization disabling based on the fact that the
deoptimization counter was incremented when materializing a frame, not when
converting code  (e.g., in recursive situations).

https://codereview.chromium.org/1559653002/diff/40001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/1559653002/diff/40001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:471: // A function with huge unoptimized code canb become
non-optimizable
On 2016/01/04 11:31:37, Florian Schneider wrote:
> s/canb/can/

Done.

https://codereview.chromium.org/1559653002/diff/40001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1559653002/diff/40001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:2816: if (a.HasCodes()) {
On 2016/01/04 11:31:38, Florian Schneider wrote:
> If you always check HasCodes before calling DisableCode, maybe add an ASSERT
> inside DisableCode()
> 
> ASSERT(HasCodes())
> 
> and remove the check for a null code array there.

What I really meant to do (and changed it to):

  CHACodeArray a(*this);
  if (FLAG_trace_deoptimization && a.HasCodes()) {
    THR_Print("Adding subclass %s\n", subclass.ToCString());
  }
  a.DisableCode();

No need to guard calling 'DisableCode', let it handle it itself (more robust).

[Florian Schneider, 2016-01-05 10:04:57]

lgtm

[srdjan, 2016-01-05 17:40:26]

Description was changed from

==========
Investigate & fix issues around usage_count and deoptimization_count

BUG=
==========

to

==========
Investigate & fix issues around usage_count and deoptimization_count

BUG=
R=fschneider@google.com

Committed:
https://github.com/dart-lang/sdk/commit/1595117898c5c40a7c3ba9c42028b9b0cd23a8a5
==========

[srdjan, 2016-01-05 17:40:27]

Committed patchset #5 (id:80001) manually as
1595117898c5c40a7c3ba9c42028b9b0cd23a8a5 (presubmit successful).