Reapply "Background finalization."

Reapply "Background finalization."

 - Fix double-finalization by marking handles that are in a finalization queue.
 - Remember external size in words instead of bytes.
 - Add SlowFinalizer test.

R=asiva@google.com

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

[rmacnak, 2016-06-22 01:16:21]

rmacnak@google.com changed reviewers:
+ asiva@google.com

[siva, 2016-06-22 04:43:30]

lgtm

https://codereview.chromium.org/2084193003/diff/1/runtime/vm/dart_api_impl_te...
File runtime/vm/dart_api_impl_test.cc (right):

https://codereview.chromium.org/2084193003/diff/1/runtime/vm/dart_api_impl_te...
runtime/vm/dart_api_impl_test.cc:2421: OS::Sleep(50);
I hope this sleep on every call won't cause timeouts in the test on some
machines. Could you ensure finalizers are happening in parallel by doing
something like

making 'i' a static variable and doing something like

while (i < 100) {
  ml.wait();
}

and in the main loop after the iteration of 100 times is done do
  ml.NotifyAll();

[rmacnak, 2016-06-22 17:50:45]

https://codereview.chromium.org/2084193003/diff/1/runtime/vm/dart_api_impl_te...
File runtime/vm/dart_api_impl_test.cc (right):

https://codereview.chromium.org/2084193003/diff/1/runtime/vm/dart_api_impl_te...
runtime/vm/dart_api_impl_test.cc:2421: OS::Sleep(50);
On 2016/06/22 04:43:29, siva wrote:
> I hope this sleep on every call won't cause timeouts in the test on some
> machines. Could you ensure finalizers are happening in parallel by doing
> something like
> 
> making 'i' a static variable and doing something like
> 
> while (i < 100) {
>   ml.wait();
> }
> 
> and in the main loop after the iteration of 100 times is done do
>   ml.NotifyAll();

I like the monitor approach better. To make it work, I am no longer having the
background finalizer enter the isolate as a helper thread, otherwise the main
thread is waiting on the background finalizer to enter a safepoint while the
background finalizer is waiting on the main thread for enough finalizers to pile
up.  PTAL.

[siva, 2016-06-22 22:55:32]

lgtm with one comment regarding removing the EnterIsolate code.

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

https://codereview.chromium.org/2084193003/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl_test.cc:2432: }
See comment below:
Isn't it sufficient to do 
while (slow_finalizers_waiting < 10) {
  ...
  ...
}
ml.NotifyAll();

https://codereview.chromium.org/2084193003/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl_test.cc:2464: }
Why is this block needed here? Isn't it sufficient to do
the NotifyAll in the last thread that calls SlowFinalizer?

https://codereview.chromium.org/2084193003/diff/20001/runtime/vm/dart_api_sta...
File runtime/vm/dart_api_state.cc (right):

https://codereview.chromium.org/2084193003/diff/20001/runtime/vm/dart_api_sta...
runtime/vm/dart_api_state.cc:29: {
You can avoid the issues with entering the isolate by transitioning to Native
before the callback in FinalizablePersistentHandle::Finalize
actually that is a bug currently.

[rmacnak, 2016-06-22 23:13:48]

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

https://codereview.chromium.org/2084193003/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl_test.cc:2432: }
On 2016/06/22 22:55:31, siva wrote:
> See comment below:
> Isn't it sufficient to do 
> while (slow_finalizers_waiting < 10) {
>   ...
>   ...
> }
> ml.NotifyAll();

Yes, that works too.

https://codereview.chromium.org/2084193003/diff/20001/runtime/vm/dart_api_sta...
File runtime/vm/dart_api_state.cc (right):

https://codereview.chromium.org/2084193003/diff/20001/runtime/vm/dart_api_sta...
runtime/vm/dart_api_state.cc:29: {
On 2016/06/22 22:55:31, siva wrote:
> You can avoid the issues with entering the isolate by transitioning to Native
> before the callback in FinalizablePersistentHandle::Finalize
> actually that is a bug currently.

Done.

[rmacnak, 2016-06-22 23:26:54]

Description was changed from

==========
Reapply "Background finalization."

 - Fix double-finalization by marking handles that are in a finalization queue.
 - Remember external size in words instead of bytes.
 - Add SlowFinalizer test.
==========

to

==========
Reapply "Background finalization."

 - Fix double-finalization by marking handles that are in a finalization queue.
 - Remember external size in words instead of bytes.
 - Add SlowFinalizer test.

R=asiva@google.com

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

[rmacnak, 2016-06-22 23:26:55]

Committed patchset #3 (id:40001) manually as
103e316f6a1b90cd52fb058ddb1efa404b9c33dc (presubmit successful).