Simpler, more consistent triggering of old GC; also, use external in policy.

Simpler, more consistent triggering of old GC; also, use external in policy.

Consistently use PageSpace::NeedsGarbageCollection:
(a) After every scavenge, instead of tracking promotion failures.
(b) When creating weak handles with external allocations.

R=iposva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=35174

[Ivan Posva, 2014-04-18 16:11:26]

Couple of questions, but generally looks good.

-Ivan

https://codereview.chromium.org/226973003/diff/80001/runtime/vm/dart_api_impl...
File runtime/vm/dart_api_impl_test.cc (right):

https://codereview.chromium.org/226973003/diff/80001/runtime/vm/dart_api_impl...
runtime/vm/dart_api_impl_test.cc:2374: EXPECT_EQ(kSmallExternalSize,
You might want to verify how much external size there was before starting the
allocation.

https://codereview.chromium.org/226973003/diff/80001/runtime/vm/dart_api_impl...
runtime/vm/dart_api_impl_test.cc:2399: Dart_Handle dart_null = Dart_True();  //
VM heap object.
Why?

[koda, 2014-04-18 16:38:18]

https://codereview.chromium.org/226973003/diff/80001/runtime/vm/dart_api_impl...
File runtime/vm/dart_api_impl_test.cc (right):

https://codereview.chromium.org/226973003/diff/80001/runtime/vm/dart_api_impl...
runtime/vm/dart_api_impl_test.cc:2374: EXPECT_EQ(kSmallExternalSize,
On 2014/04/18 16:11:26, Ivan Posva wrote:
> You might want to verify how much external size there was before starting the
> allocation.

Done.

https://codereview.chromium.org/226973003/diff/80001/runtime/vm/dart_api_impl...
runtime/vm/dart_api_impl_test.cc:2399: Dart_Handle dart_null = Dart_True();  //
VM heap object.
On 2014/04/18 16:11:26, Ivan Posva wrote:
> Why?

I assume you mean "Why the change from 'null' to 'true'?"

I learned that 'null' is (counter-intuitively) considered always unreachable
during GC. Therefore, using it as a referent leads to strange behavior. True
just serves as an example of a VM heap object. Although VM heap object referents
are unlikely in practice, they constitute a corner case that should be tested
(or explicitly forbidden).

I now also renamed this local variable to the more suitable 'dart_true'.

[Ivan Posva, 2014-04-18 16:41:21]

LGTM -ip

https://codereview.chromium.org/226973003/diff/100001/runtime/vm/dart_api_imp...
File runtime/vm/dart_api_impl_test.cc (right):

https://codereview.chromium.org/226973003/diff/100001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl_test.cc:2400: Dart_Handle dart_true = Dart_True();  //
VM heap object.
Better now! And thanks for the explanation.

[koda, 2014-04-18 18:23:58]

Committed patchset #6 manually as r35174 (presubmit successful).