VM(CoreLib): Work around JIT optimization issue revealed by cec963f028198ec5871840491ba78b096ad0b819

VM(CoreLib): Work around JIT optimization issue revealed by cec963f028198ec5871840491ba78b096ad0b819

Array Bounds Check Generalization pass attempts to hoist CheckArrayBound
instructions out of loops by generalizing them: for example in the loop
like


  for (var i = 0; i < L; i++) {
    // a[i]
    CheckArrayBound(i, a.length)
    LoadIndexed(a, i)
  }

we hoist CheckArrayBound(i, a.length) by turning it into
CheckArrayBound(L - 1, a.length):

  CheckArrayBound(L - 1, a.length)
  for (var i = 0; i < L; i++) {
    // a[i]
    LoadIndexed(a, i)
  }

However this leads to deoptimizations if the loop never executes: e.g. if
L = 0 then L - 1 is -1 which does not pass generalized bounds check.

We prevent repeated deoptimizations by disabling this optimization if any
of the generalized bounds checks deoptimizes. However this does not work
as intended because we only check this flag on an outermost function, so
if function with a problematic bounds check get inlined into a lot of places
it ends up causing a lot of deoptimizations in different places.

This is why cec963f028198ec5871840491ba78b096ad0b819 caused performance issues
in dartdoc: _GrowableList._grow is now called with length == 0 whenever we
add an element to an empty list. This causes deoptimization in a lot of different
functions in dartdoc because _GrowableList._grow ends up inlined into a lot
of different places.

Fixes https://github.com/dart-lang/sdk/issues/30090

This CL will be reverted once a better solution for the underlying problem is in place (tracked by https://github.com/dart-lang/sdk/issues/30102)

BUG=
R=askesc@google.com, erikcorry@google.com

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

[Vyacheslav Egorov (Google), 2017-07-07 12:19:40]

vegorov@google.com changed reviewers:
+ askesc@google.com, erikcorry@google.com

[Vyacheslav Egorov (Google), 2017-07-07 12:19:40]

PTAL

[Vyacheslav Egorov (Google), 2017-07-07 12:20:01]

Description was changed from

==========
VM(CoreLib): Work around JIT optimization issue revealed by
cec963f028198ec5871840491ba78b096ad0b819

Array Bounds Check Generalization pass attemps to hoist CheckArrayBound
instructions out of the loops by generalizing them: for example in the loop
like


  for (var i = 0; i < L; i++) {
    // a[i]
    CheckArrayBound(i, a.length)
    LoadIndexed(a, i)
  }

we hoist CheckArrayBound(i, a.length) by turning it into
CheckArrayBound(L - 1, a.length):

  CheckArrayBound(L - 1, a.length)
  for (var i = 0; i < L; i++) {
    // a[i]
    LoadIndexed(a, i)
  }

However this leads to deoptimizations if the loop never executes: e.g. if
L = 0 then L - 1 is -1 which does not pass generalized bounds check.

We prevent repeated deoptimizations by disabling this optimization if any
of the generalized bounds checks deoptimizes. However this does not work
as intended because we only check this flag on an outermost function, so
if function with a problematic bounds check get inlined into a lot of places
it ends up causing a lot of deoptimizations in different places.

This is why cec963f028198ec5871840491ba78b096ad0b819 caused performance issues
in dartdoc: _GrowableList._grow is now called with length == 0 whenever we
add an element to an empty list. This causes deoptimization in a lot of
different
functions in dartdoc because _GrowableList._grow ends up inlined into a lot
of different places.

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

to

==========
VM(CoreLib): Work around JIT optimization issue revealed by
cec963f028198ec5871840491ba78b096ad0b819

Array Bounds Check Generalization pass attemps to hoist CheckArrayBound
instructions out of the loops by generalizing them: for example in the loop
like


  for (var i = 0; i < L; i++) {
    // a[i]
    CheckArrayBound(i, a.length)
    LoadIndexed(a, i)
  }

we hoist CheckArrayBound(i, a.length) by turning it into
CheckArrayBound(L - 1, a.length):

  CheckArrayBound(L - 1, a.length)
  for (var i = 0; i < L; i++) {
    // a[i]
    LoadIndexed(a, i)
  }

However this leads to deoptimizations if the loop never executes: e.g. if
L = 0 then L - 1 is -1 which does not pass generalized bounds check.

We prevent repeated deoptimizations by disabling this optimization if any
of the generalized bounds checks deoptimizes. However this does not work
as intended because we only check this flag on an outermost function, so
if function with a problematic bounds check get inlined into a lot of places
it ends up causing a lot of deoptimizations in different places.

This is why cec963f028198ec5871840491ba78b096ad0b819 caused performance issues
in dartdoc: _GrowableList._grow is now called with length == 0 whenever we
add an element to an empty list. This causes deoptimization in a lot of
different
functions in dartdoc because _GrowableList._grow ends up inlined into a lot
of different places.

Fixes https://github.com/dart-lang/sdk/issues/30090

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

[erikcorry, 2017-07-07 12:22:40]

lgtm

[Vyacheslav Egorov (Google), 2017-07-07 12:30:26]

Description was changed from

==========
VM(CoreLib): Work around JIT optimization issue revealed by
cec963f028198ec5871840491ba78b096ad0b819

Array Bounds Check Generalization pass attemps to hoist CheckArrayBound
instructions out of the loops by generalizing them: for example in the loop
like


  for (var i = 0; i < L; i++) {
    // a[i]
    CheckArrayBound(i, a.length)
    LoadIndexed(a, i)
  }

we hoist CheckArrayBound(i, a.length) by turning it into
CheckArrayBound(L - 1, a.length):

  CheckArrayBound(L - 1, a.length)
  for (var i = 0; i < L; i++) {
    // a[i]
    LoadIndexed(a, i)
  }

However this leads to deoptimizations if the loop never executes: e.g. if
L = 0 then L - 1 is -1 which does not pass generalized bounds check.

We prevent repeated deoptimizations by disabling this optimization if any
of the generalized bounds checks deoptimizes. However this does not work
as intended because we only check this flag on an outermost function, so
if function with a problematic bounds check get inlined into a lot of places
it ends up causing a lot of deoptimizations in different places.

This is why cec963f028198ec5871840491ba78b096ad0b819 caused performance issues
in dartdoc: _GrowableList._grow is now called with length == 0 whenever we
add an element to an empty list. This causes deoptimization in a lot of
different
functions in dartdoc because _GrowableList._grow ends up inlined into a lot
of different places.

Fixes https://github.com/dart-lang/sdk/issues/30090

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

to

==========
VM(CoreLib): Work around JIT optimization issue revealed by
cec963f028198ec5871840491ba78b096ad0b819

Array Bounds Check Generalization pass attemps to hoist CheckArrayBound
instructions out of the loops by generalizing them: for example in the loop
like


  for (var i = 0; i < L; i++) {
    // a[i]
    CheckArrayBound(i, a.length)
    LoadIndexed(a, i)
  }

we hoist CheckArrayBound(i, a.length) by turning it into
CheckArrayBound(L - 1, a.length):

  CheckArrayBound(L - 1, a.length)
  for (var i = 0; i < L; i++) {
    // a[i]
    LoadIndexed(a, i)
  }

However this leads to deoptimizations if the loop never executes: e.g. if
L = 0 then L - 1 is -1 which does not pass generalized bounds check.

We prevent repeated deoptimizations by disabling this optimization if any
of the generalized bounds checks deoptimizes. However this does not work
as intended because we only check this flag on an outermost function, so
if function with a problematic bounds check get inlined into a lot of places
it ends up causing a lot of deoptimizations in different places.

This is why cec963f028198ec5871840491ba78b096ad0b819 caused performance issues
in dartdoc: _GrowableList._grow is now called with length == 0 whenever we
add an element to an empty list. This causes deoptimization in a lot of
different
functions in dartdoc because _GrowableList._grow ends up inlined into a lot
of different places.

Fixes https://github.com/dart-lang/sdk/issues/30090

This CL will be reverted once a better solution for the underlying problem is in
place (tracked by https://github.com/dart-lang/sdk/issues/30102)

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

[Vyacheslav Egorov (Google), 2017-07-07 12:30:46]

Description was changed from

==========
VM(CoreLib): Work around JIT optimization issue revealed by
cec963f028198ec5871840491ba78b096ad0b819

Array Bounds Check Generalization pass attemps to hoist CheckArrayBound
instructions out of the loops by generalizing them: for example in the loop
like


  for (var i = 0; i < L; i++) {
    // a[i]
    CheckArrayBound(i, a.length)
    LoadIndexed(a, i)
  }

we hoist CheckArrayBound(i, a.length) by turning it into
CheckArrayBound(L - 1, a.length):

  CheckArrayBound(L - 1, a.length)
  for (var i = 0; i < L; i++) {
    // a[i]
    LoadIndexed(a, i)
  }

However this leads to deoptimizations if the loop never executes: e.g. if
L = 0 then L - 1 is -1 which does not pass generalized bounds check.

We prevent repeated deoptimizations by disabling this optimization if any
of the generalized bounds checks deoptimizes. However this does not work
as intended because we only check this flag on an outermost function, so
if function with a problematic bounds check get inlined into a lot of places
it ends up causing a lot of deoptimizations in different places.

This is why cec963f028198ec5871840491ba78b096ad0b819 caused performance issues
in dartdoc: _GrowableList._grow is now called with length == 0 whenever we
add an element to an empty list. This causes deoptimization in a lot of
different
functions in dartdoc because _GrowableList._grow ends up inlined into a lot
of different places.

Fixes https://github.com/dart-lang/sdk/issues/30090

This CL will be reverted once a better solution for the underlying problem is in
place (tracked by https://github.com/dart-lang/sdk/issues/30102)

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

to

==========
VM(CoreLib): Work around JIT optimization issue revealed by
cec963f028198ec5871840491ba78b096ad0b819

Array Bounds Check Generalization pass attempts to hoist CheckArrayBound
instructions out of loops by generalizing them: for example in the loop
like


  for (var i = 0; i < L; i++) {
    // a[i]
    CheckArrayBound(i, a.length)
    LoadIndexed(a, i)
  }

we hoist CheckArrayBound(i, a.length) by turning it into
CheckArrayBound(L - 1, a.length):

  CheckArrayBound(L - 1, a.length)
  for (var i = 0; i < L; i++) {
    // a[i]
    LoadIndexed(a, i)
  }

However this leads to deoptimizations if the loop never executes: e.g. if
L = 0 then L - 1 is -1 which does not pass generalized bounds check.

We prevent repeated deoptimizations by disabling this optimization if any
of the generalized bounds checks deoptimizes. However this does not work
as intended because we only check this flag on an outermost function, so
if function with a problematic bounds check get inlined into a lot of places
it ends up causing a lot of deoptimizations in different places.

This is why cec963f028198ec5871840491ba78b096ad0b819 caused performance issues
in dartdoc: _GrowableList._grow is now called with length == 0 whenever we
add an element to an empty list. This causes deoptimization in a lot of
different
functions in dartdoc because _GrowableList._grow ends up inlined into a lot
of different places.

Fixes https://github.com/dart-lang/sdk/issues/30090

This CL will be reverted once a better solution for the underlying problem is in
place (tracked by https://github.com/dart-lang/sdk/issues/30102)

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

[Aske Simon Christensen, 2017-07-07 12:30:59]

lgtm

[Vyacheslav Egorov (Google), 2017-07-07 12:34:54]

Description was changed from

==========
VM(CoreLib): Work around JIT optimization issue revealed by
cec963f028198ec5871840491ba78b096ad0b819

Array Bounds Check Generalization pass attempts to hoist CheckArrayBound
instructions out of loops by generalizing them: for example in the loop
like


  for (var i = 0; i < L; i++) {
    // a[i]
    CheckArrayBound(i, a.length)
    LoadIndexed(a, i)
  }

we hoist CheckArrayBound(i, a.length) by turning it into
CheckArrayBound(L - 1, a.length):

  CheckArrayBound(L - 1, a.length)
  for (var i = 0; i < L; i++) {
    // a[i]
    LoadIndexed(a, i)
  }

However this leads to deoptimizations if the loop never executes: e.g. if
L = 0 then L - 1 is -1 which does not pass generalized bounds check.

We prevent repeated deoptimizations by disabling this optimization if any
of the generalized bounds checks deoptimizes. However this does not work
as intended because we only check this flag on an outermost function, so
if function with a problematic bounds check get inlined into a lot of places
it ends up causing a lot of deoptimizations in different places.

This is why cec963f028198ec5871840491ba78b096ad0b819 caused performance issues
in dartdoc: _GrowableList._grow is now called with length == 0 whenever we
add an element to an empty list. This causes deoptimization in a lot of
different
functions in dartdoc because _GrowableList._grow ends up inlined into a lot
of different places.

Fixes https://github.com/dart-lang/sdk/issues/30090

This CL will be reverted once a better solution for the underlying problem is in
place (tracked by https://github.com/dart-lang/sdk/issues/30102)

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

to

==========
VM(CoreLib): Work around JIT optimization issue revealed by
cec963f028198ec5871840491ba78b096ad0b819

Array Bounds Check Generalization pass attempts to hoist CheckArrayBound
instructions out of loops by generalizing them: for example in the loop
like


  for (var i = 0; i < L; i++) {
    // a[i]
    CheckArrayBound(i, a.length)
    LoadIndexed(a, i)
  }

we hoist CheckArrayBound(i, a.length) by turning it into
CheckArrayBound(L - 1, a.length):

  CheckArrayBound(L - 1, a.length)
  for (var i = 0; i < L; i++) {
    // a[i]
    LoadIndexed(a, i)
  }

However this leads to deoptimizations if the loop never executes: e.g. if
L = 0 then L - 1 is -1 which does not pass generalized bounds check.

We prevent repeated deoptimizations by disabling this optimization if any
of the generalized bounds checks deoptimizes. However this does not work
as intended because we only check this flag on an outermost function, so
if function with a problematic bounds check get inlined into a lot of places
it ends up causing a lot of deoptimizations in different places.

This is why cec963f028198ec5871840491ba78b096ad0b819 caused performance issues
in dartdoc: _GrowableList._grow is now called with length == 0 whenever we
add an element to an empty list. This causes deoptimization in a lot of
different
functions in dartdoc because _GrowableList._grow ends up inlined into a lot
of different places.

Fixes https://github.com/dart-lang/sdk/issues/30090

This CL will be reverted once a better solution for the underlying problem is in
place (tracked by https://github.com/dart-lang/sdk/issues/30102)

BUG=
R=askesc@google.com, erikcorry@google.com

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

[Vyacheslav Egorov (Google), 2017-07-07 12:34:56]

Committed patchset #1 (id:1) manually as
d24040af02d13264c198ced6774c5ce32e75188a (presubmit successful).