Some fixes to GC heuristics.

Some fixes to GC heuristics.

The loop intended to do a binary search for the best heap growth size
did not have any variables in it that changed the decision, so it would
make the same decision (up or down) every time around the loop.

R=kasperl@google.com, vegorov@google.com
BUG=

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

[kasperl, 2017-04-21 10:11:28]

kasperl@google.com changed reviewers:
+ kasperl@google.com

[kasperl, 2017-04-21 10:11:28]

DBC:

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

https://codereview.chromium.org/2829853007/diff/1/runtime/vm/pages.cc#newcode...
runtime/vm/pages.cc:1220: // It makes no sense to expect that each kb alllocated
will cause more than
alllocated -> allocated

[erikcorry, 2017-04-21 10:34:27]

Description was changed from

==========
Some fixes to GC heuristics.

The loop intended to do a binary search for the best heap growth size
did not have any variables in it that changed the decision, so it would
make the same decision (up or down) every time around the loop.

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

to

==========
Some fixes to GC heuristics.

The loop intended to do a binary search for the best heap growth size
did not have any variables in it that changed the decision, so it would
make the same decision (up or down) every time around the loop.

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

[erikcorry, 2017-04-21 10:34:27]

erikcorry@google.com changed reviewers:
+ asiva@google.com
- vegorov@google.com

[erikcorry, 2017-04-21 15:51:06]

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

https://codereview.chromium.org/2829853007/diff/1/runtime/vm/pages.cc#newcode...
runtime/vm/pages.cc:1220: // It makes no sense to expect that each kb alllocated
will cause more than
On 2017/04/21 10:11:28, kasperl wrote:
> alllocated -> allocated

Done.

[erikcorry, 2017-04-24 08:20:07]

PTAL

[erikcorry, 2017-04-24 08:33:10]

BTW it was while investigating this that I saw the back-to-back GC issue in
https://github.com/dart-lang/sdk/issues/29415

While it's pretty clear what is going wrong there, I wasn't sure what the best
way to fix it is, so I made the bug in the hope of getting some feedback.

[kasperl, 2017-04-27 06:58:17]

LGTM.

https://codereview.chromium.org/2829853007/diff/20001/runtime/vm/pages.cc
File runtime/vm/pages.cc (right):

https://codereview.chromium.org/2829853007/diff/20001/runtime/vm/pages.cc#new...
runtime/vm/pages.cc:1214: // G = kA, and estimate k from the previous cycle.
Nit: Can we rename 'k' to garbage_ratio, ratio, or 'r'? To me, k is a bit
overloaded (kilo?) and is usually used for integer constants (like n).

https://codereview.chromium.org/2829853007/diff/20001/runtime/vm/pages.cc#new...
runtime/vm/pages.cc:1225: const int garbage_ratio = static_cast<int>(k * 100);
Maybe we wouldn't need this named constant if 'k' had a better name?

https://codereview.chromium.org/2829853007/diff/20001/runtime/vm/pages.cc#new...
runtime/vm/pages.cc:1239: if (garbage_ratio == 0) {
Express through 'k' instead?

[erikcorry, 2017-04-27 13:04:41]

Description was changed from

==========
Some fixes to GC heuristics.

The loop intended to do a binary search for the best heap growth size
did not have any variables in it that changed the decision, so it would
make the same decision (up or down) every time around the loop.

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

to

==========
Some fixes to GC heuristics.

The loop intended to do a binary search for the best heap growth size
did not have any variables in it that changed the decision, so it would
make the same decision (up or down) every time around the loop.

R=kasperl@google.com, vegorov@google.com
BUG=

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

[erikcorry, 2017-04-27 13:04:43]

Committed patchset #2 (id:20001) manually as
2a9cb6bbb730c544c8c12f8713fb8105a0bd7ca7 (presubmit successful).