Disable heap growth during weak handle processing unit test in order to avoid triggering of old spa…

Invoke 'Scavenge' in the weak handle processing unit tests and not 'CollectGarbage' in order to avoid triggering of old space mark sweep in certain situations.
Committed: https://code.google.com/p/dart/source/detail?r=9535

[cshapiro, 2012-07-09 20:15:27]

Growth control should be off until it is explicitly enabled.  Is there anyway we
can have the isolates used for the tests be started in a state without the
growth control enabled?

In general, I do not like having a switch to disable growth control as it is
difficult to describe the interaction between the already accumulated statistics
and the behavior after the growth control is re-enabled.  If there is really no
other way to do this, I would prefer that the enabling be changed to zero out
the state of the page space controller to ensure that the time and spaced
consumed while the growth policy was suspended is not erroneously accounted.

[siva, 2012-07-09 20:44:38]

Not sure if it is a good idea to turn off growth control on all tests as you
might have situations that need this to be tested.
I agree that the stats get hosed when you disable it in mid-stream, maybe I will
abandon this way of fixing this test and instead just invoke Scavenge in these
tests when New space garbage collection needs to be done.

On 2012/07/09 20:15:27, cshapiro wrote:
> Growth control should be off until it is explicitly enabled.  Is there anyway
we
> can have the isolates used for the tests be started in a state without the
> growth control enabled?
> 
> In general, I do not like having a switch to disable growth control as it is
> difficult to describe the interaction between the already accumulated
statistics
> and the behavior after the growth control is re-enabled.  If there is really
no
> other way to do this, I would prefer that the enabling be changed to zero out
> the state of the page space controller to ensure that the time and spaced
> consumed while the growth policy was suspended is not erroneously accounted.

[cshapiro, 2012-07-10 21:15:38]

https://chromiumcodereview.appspot.com/10748006/diff/1005/vm/dart_api_impl_te...
File vm/dart_api_impl_test.cc (right):

https://chromiumcodereview.appspot.com/10748006/diff/1005/vm/dart_api_impl_te...
vm/dart_api_impl_test.cc:1310: // create an object in new space.
Presumably the periods are being added to make these more like real sentences. 
If so, the first letter should be capitalized as well.

https://chromiumcodereview.appspot.com/10748006/diff/1005/vm/dart_api_impl_te...
vm/dart_api_impl_test.cc:1334: Isolate::Current()->heap()->Scavenge();
I think it would be helpful to expand the comment to note why Scavenge is being
invoked directly rather than indirectly through CollectGarbage.  This is noted
in the check-in comment, but it is sufficiently subtle to deserve to be kept
in-line.  Perhaps this will be unnecessary if the helper class we spoke about
off-line has a sufficiently self describing name.

https://chromiumcodereview.appspot.com/10748006/diff/1005/vm/dart_api_impl_te...
vm/dart_api_impl_test.cc:1372: Isolate::Current()->heap()->Scavenge();
Ditto.

[siva, 2012-07-10 21:27:15]

Addressed comments.

PTAL.

https://chromiumcodereview.appspot.com/10748006/diff/1005/vm/dart_api_impl_te...
File vm/dart_api_impl_test.cc (right):

https://chromiumcodereview.appspot.com/10748006/diff/1005/vm/dart_api_impl_te...
vm/dart_api_impl_test.cc:1310: // create an object in new space.
On 2012/07/10 21:15:39, cshapiro wrote:
> Presumably the periods are being added to make these more like real sentences.

> If so, the first letter should be capitalized as well.

Done.

https://chromiumcodereview.appspot.com/10748006/diff/1005/vm/dart_api_impl_te...
vm/dart_api_impl_test.cc:1334: Isolate::Current()->heap()->Scavenge();
As discussed I have added a class called GCTestHelper and added the comment over
there.

On 2012/07/10 21:15:39, cshapiro wrote:
> I think it would be helpful to expand the comment to note why Scavenge is
being
> invoked directly rather than indirectly through CollectGarbage.  This is noted
> in the check-in comment, but it is sufficiently subtle to deserve to be kept
> in-line.  Perhaps this will be unnecessary if the helper class we spoke about
> off-line has a sufficiently self describing name.

https://chromiumcodereview.appspot.com/10748006/diff/1005/vm/dart_api_impl_te...
vm/dart_api_impl_test.cc:1372: Isolate::Current()->heap()->Scavenge();
Ditto.

On 2012/07/10 21:15:39, cshapiro wrote:
> Ditto.

[cshapiro, 2012-07-10 22:16:43]

lgtm

https://chromiumcodereview.appspot.com/10748006/diff/18001/vm/dart_api_impl_t...
File vm/dart_api_impl_test.cc (right):

https://chromiumcodereview.appspot.com/10748006/diff/18001/vm/dart_api_impl_t...
vm/dart_api_impl_test.cc:1299: // The normal call to Collectgarbage(heap::kNew)
could potentially trigger
More shift key

  CollectGarbage(Heap::kNew)
---------^       ^
-----------------|

https://chromiumcodereview.appspot.com/10748006/diff/18001/vm/dart_api_impl_t...
vm/dart_api_impl_test.cc:1304: static void CollectNewGen(Heap::ApiCallbacks
api_callbacks) {
Not a big deal, but CollectNewSpace is probably a better name since we
internally refer to the partitions as spaces rather than generations.

[siva, 2012-07-10 22:57:50]

https://chromiumcodereview.appspot.com/10748006/diff/18001/vm/dart_api_impl_t...
File vm/dart_api_impl_test.cc (right):

https://chromiumcodereview.appspot.com/10748006/diff/18001/vm/dart_api_impl_t...
vm/dart_api_impl_test.cc:1299: // The normal call to Collectgarbage(heap::kNew)
could potentially trigger
On 2012/07/10 22:16:43, cshapiro wrote:
> More shift key
> 
>   CollectGarbage(Heap::kNew)
> ---------^       ^
> -----------------|

Done.

https://chromiumcodereview.appspot.com/10748006/diff/18001/vm/dart_api_impl_t...
vm/dart_api_impl_test.cc:1304: static void CollectNewGen(Heap::ApiCallbacks
api_callbacks) {
On 2012/07/10 22:16:43, cshapiro wrote:
> Not a big deal, but CollectNewSpace is probably a better name since we
> internally refer to the partitions as spaces rather than generations.

Done.