Fall back to megamorphic stub when dispatching calls to rare classes.

Fall back to megamorphic stub when dispatching calls to rare classes.

Previously we were converting very polymorphic calls that always end up
in the same implementation into a check-class instruction and a (usually
inlined) direct call.  This works well in artificial benchmarks like
the Megamorphic suite, but tends to cause repeated deopts on large real
programs like dart2js, where new classes arrive after a while, triggering
the check-class instruction to deopt.  After a few deopts, the function
is no longer eligible for optimization, resulting in huge slowdowns.

Instead with this change we can switch to the normal machinery for polymorphic
calls with more than one target.  This has the advantage that we use a
megamorphic stub call to catch rare classes, rather than deopting.
Dart2js is speeded up by a few percent on average and avoids the big
performance potholes described in https://github.com/dart-lang/sdk/issues/29302

Also reverts a less effective workaround for 29302.

R=vegorov@google.com
BUG=29302
Committed: https://github.com/dart-lang/sdk/commit/0856863355595088aac6a519a77a2e6987bf26cf

[Vyacheslav Egorov (Google), 2017-04-19 08:24:00]

LGTM

Can we update artificial Megamorphic benchmark to exhibit similar behavior?
(e.g. as an additional case)

(e.g. classes come to callsite gradually - causing deopts - instead of being
seen all at once by the optimizer).

https://codereview.chromium.org/2829603002/diff/1/runtime/vm/jit_optimizer.cc
File runtime/vm/jit_optimizer.cc (right):

https://codereview.chromium.org/2829603002/diff/1/runtime/vm/jit_optimizer.cc...
runtime/vm/jit_optimizer.cc:1621: const int max_cids = (op_kind == Token::kEQ)
It might be good to put the reasoning here to inform future generations

[erikcorry, 2017-04-19 10:57:06]

Description was changed from

==========
Fall back to megamorphic stub when dispatching calls to rare classes.

Previously we were converting very polymorphic calls that always end up
in the same implementation into a check-class instruction and a (usually
inlined) direct call.  This works well in artificial benchmarks like
the Megamorphic suite, but tends to cause repeated deopts on large real
programs like dart2js, where new classes arrive after a while, triggering
the check-class instruction to deopt.  After a few deopts, the function
is no longer eligible for optimization, resulting in huge slowdowns.

Instead with this change we can switch to the normal machinery for polymorphic
calls with more than one target.  This has the advantage that we use a
megamorphic stub call to catch rare classes, rather than deopting. This slows
down the artificial benchmarks in the Megamorphic suite, but they are still
faster than before
https://github.com/dart-lang/sdk/commit/ae8e6705c0e1dd624ec45843879294199c11e368
Dart2js is speeded up by a few percent on average and avoids the big
performance potholes described in https://github.com/dart-lang/sdk/issues/29302

Also reverts a less effective workaround for 29302.

R=vegorov@google.com
BUG=29302
==========

to

==========
Fall back to megamorphic stub when dispatching calls to rare classes.

Previously we were converting very polymorphic calls that always end up
in the same implementation into a check-class instruction and a (usually
inlined) direct call.  This works well in artificial benchmarks like
the Megamorphic suite, but tends to cause repeated deopts on large real
programs like dart2js, where new classes arrive after a while, triggering
the check-class instruction to deopt.  After a few deopts, the function
is no longer eligible for optimization, resulting in huge slowdowns.

Instead with this change we can switch to the normal machinery for polymorphic
calls with more than one target.  This has the advantage that we use a
megamorphic stub call to catch rare classes, rather than deopting.
Dart2js is speeded up by a few percent on average and avoids the big
performance potholes described in https://github.com/dart-lang/sdk/issues/29302

Also reverts a less effective workaround for 29302.

R=vegorov@google.com
BUG=29302
==========

[erikcorry, 2017-04-19 13:15:41]

https://codereview.chromium.org/2829603002/diff/1/runtime/vm/jit_optimizer.cc
File runtime/vm/jit_optimizer.cc (right):

https://codereview.chromium.org/2829603002/diff/1/runtime/vm/jit_optimizer.cc...
runtime/vm/jit_optimizer.cc:1621: const int max_cids = (op_kind == Token::kEQ)
On 2017/04/19 08:24:00, Vyacheslav Egorov (Google) wrote:
> It might be good to put the reasoning here to inform future generations

Done.

[Vyacheslav Egorov (Google), 2017-04-19 13:21:55]

still lgtm

[erikcorry, 2017-04-19 13:25:30]

Description was changed from

==========
Fall back to megamorphic stub when dispatching calls to rare classes.

Previously we were converting very polymorphic calls that always end up
in the same implementation into a check-class instruction and a (usually
inlined) direct call.  This works well in artificial benchmarks like
the Megamorphic suite, but tends to cause repeated deopts on large real
programs like dart2js, where new classes arrive after a while, triggering
the check-class instruction to deopt.  After a few deopts, the function
is no longer eligible for optimization, resulting in huge slowdowns.

Instead with this change we can switch to the normal machinery for polymorphic
calls with more than one target.  This has the advantage that we use a
megamorphic stub call to catch rare classes, rather than deopting.
Dart2js is speeded up by a few percent on average and avoids the big
performance potholes described in https://github.com/dart-lang/sdk/issues/29302

Also reverts a less effective workaround for 29302.

R=vegorov@google.com
BUG=29302
==========

to

==========
Fall back to megamorphic stub when dispatching calls to rare classes.

Previously we were converting very polymorphic calls that always end up
in the same implementation into a check-class instruction and a (usually
inlined) direct call.  This works well in artificial benchmarks like
the Megamorphic suite, but tends to cause repeated deopts on large real
programs like dart2js, where new classes arrive after a while, triggering
the check-class instruction to deopt.  After a few deopts, the function
is no longer eligible for optimization, resulting in huge slowdowns.

Instead with this change we can switch to the normal machinery for polymorphic
calls with more than one target.  This has the advantage that we use a
megamorphic stub call to catch rare classes, rather than deopting.
Dart2js is speeded up by a few percent on average and avoids the big
performance potholes described in https://github.com/dart-lang/sdk/issues/29302

Also reverts a less effective workaround for 29302.

R=vegorov@google.com
BUG=29302

Committed:
https://github.com/dart-lang/sdk/commit/0856863355595088aac6a519a77a2e6987bf26cf
==========

[erikcorry, 2017-04-19 13:25:31]

Committed patchset #3 (id:40001) manually as
0856863355595088aac6a519a77a2e6987bf26cf (presubmit successful).

[Cutch, 2017-04-19 15:48:04]

How does this change affect flutter applications?

[erikcorry, 2017-04-20 07:04:28]

On 2017/04/19 15:48:04, Cutch wrote:
> How does this change affect flutter applications?

I don't expect any changes.  Does Golem have any benchmarks that reflect Flutter
apps?

It makes things more like they were a few weeks ago for a few very "polymorphic"
call sites that have only one target (ie they are not really polymorphic, it's
just a very wide class hierarchy).  Until recently they were generated as a call
to the megamorphic stub which did a hash table lookup on every invocation only
to always call the same method.  Until yesterday that was replaced with a class
ID check and a direct call (often inlined).  Now you get the same sort of thing,
but there's a call to the stub instead of a deopt if a new or rare receiver
class is seen.