- Add different types for persistent and weak persistent handles

- Add different types for persistent and weak persistent handles
  in the Dart C API.
- Adapt code in the runtime.

R=asiva@google.com

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

[Ivan Posva, 2013-05-22 22:49:27]

Suggested API (needs cleanup and documentation) for addition of
Dart_PersistentHandle and Dart_WeakPersistentHandle.

-Ivan

P.S. Still failing tests.

[Anton Muhin, 2013-05-23 17:18:03]

thanks!

https://codereview.chromium.org/15772005/diff/1/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/15772005/diff/1/runtime/include/dart_api.h#ne...
runtime/include/dart_api.h:1766: * Returns a TypedData object which references
an external data array.
nit: document needs an update

[Ivan Posva, 2013-05-25 00:36:50]

Anton,

This version now passes all tests. Please let us know whether this revised API
works for you and I will finish the documentation comments and fix the lint
errors.

Thanks,
-Ivan

[Anton Muhin, 2013-05-27 15:45:49]

I've got a CL which switches Dartium to new API

https://codereview.chromium.org/15772005/diff/24001/runtime/include/dart_api.h
File runtime/include/dart_api.h (left):

https://codereview.chromium.org/15772005/diff/24001/runtime/include/dart_api....
runtime/include/dart_api.h:1779: DART_EXPORT Dart_Handle
Dart_ExternalTypedDataGetPeer(Dart_Handle object,
there still should be a way to fetch peer object associated with external typed
data.

For now I use Dart_SetPeer/Dart_GetPeer, but I recall that assumption was they
are not used too much which might be not the case for typed data.

https://codereview.chromium.org/15772005/diff/24001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/15772005/diff/24001/runtime/include/dart_api....
runtime/include/dart_api.h:473: Dart_WeakPersistentHandle* keys,
there was one fancy trick in our code which currently not applicable: we need to
render some weak persistent handles as strong for GC.  We did it by creating a
set with strong persistent handle.

I implemented trivial workaround for now: I create a weak persistent handle for
an object held by strong persistent handle, but if we can direct this case in
API directly, that'd be great.

One simple approach would be either add a function or add a special semantics
for references set with empty key sets.

[siva, 2013-05-28 17:37:17]

lgtm

https://codereview.chromium.org/15772005/diff/24001/runtime/bin/secure_socket.cc
File runtime/bin/secure_socket.cc (right):

https://codereview.chromium.org/15772005/diff/24001/runtime/bin/secure_socket...
runtime/bin/secure_socket.cc:342: ASSERT(bad_certificate_callback_ != NULL);
Why has this been changed to eagerly create bad_certificate_callback_
instead of the way it was being done before?

https://codereview.chromium.org/15772005/diff/24001/runtime/lib/mirrors.cc
File runtime/lib/mirrors.cc (right):

https://codereview.chromium.org/15772005/diff/24001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:147: ASSERT(!Dart_IsError(handle));
ASSERT(handle != NULL);

https://codereview.chromium.org/15772005/diff/24001/runtime/vm/dart.cc
File runtime/vm/dart.cc (right):

https://codereview.chromium.org/15772005/diff/24001/runtime/vm/dart.cc#newcod...
runtime/vm/dart.cc:136: 
At some point we should add Dart::Cleanup function to undo all
the stuff that was done here.

https://codereview.chromium.org/15772005/diff/24001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (left):

https://codereview.chromium.org/15772005/diff/24001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:2712: 
Why is this not needed anymore?

https://codereview.chromium.org/15772005/diff/24001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/15772005/diff/24001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:203: Dart_Handle Api::Null(Isolate* isolate) {
Is the isolate parameter still needed for these handle accessors?
These handles are coming from the vm isolate.

[Ivan Posva, 2013-05-28 21:12:20]

https://codereview.chromium.org/15772005/diff/24001/runtime/bin/secure_socket.cc
File runtime/bin/secure_socket.cc (right):

https://codereview.chromium.org/15772005/diff/24001/runtime/bin/secure_socket...
runtime/bin/secure_socket.cc:342: ASSERT(bad_certificate_callback_ != NULL);
On 2013/05/28 17:37:18, siva wrote:
> Why has this been changed to eagerly create bad_certificate_callback_
> instead of the way it was being done before?

It was more regular this way by avoiding extra (bad_certificate_callback() ==
NULL) tests, especially in the accessor it does not have to check for NULL with
this pattern.

https://codereview.chromium.org/15772005/diff/24001/runtime/include/dart_api.h
File runtime/include/dart_api.h (left):

https://codereview.chromium.org/15772005/diff/24001/runtime/include/dart_api....
runtime/include/dart_api.h:1779: DART_EXPORT Dart_Handle
Dart_ExternalTypedDataGetPeer(Dart_Handle object,
On 2013/05/27 15:45:49, Anton Muhin wrote:
> there still should be a way to fetch peer object associated with external
typed
> data.
> 
> For now I use Dart_SetPeer/Dart_GetPeer, but I recall that assumption was they
> are not used too much which might be not the case for typed data.

For now, please use Dart_SetPeer and Dart_GetPeer if you need to have access to
a peer value.

https://codereview.chromium.org/15772005/diff/24001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/15772005/diff/24001/runtime/include/dart_api....
runtime/include/dart_api.h:473: Dart_WeakPersistentHandle* keys,
On 2013/05/27 15:45:49, Anton Muhin wrote:
> there was one fancy trick in our code which currently not applicable: we need
to
> render some weak persistent handles as strong for GC.  We did it by creating a
> set with strong persistent handle.
> 
> I implemented trivial workaround for now: I create a weak persistent handle
for
> an object held by strong persistent handle, but if we can direct this case in
> API directly, that'd be great.
> 
> One simple approach would be either add a function or add a special semantics
> for references set with empty key sets.

I am not sure I understand your usage of this API function, but to me it sounds
like this:
Occasionally you have a bunch of weak handles, which need to be preserved during
a GC. You used to create one strong persistent handle, use that as the "keys"
and the to-be-preserved weak handles as the values. Do I understand that
correctly?

If that is the case, then I suggest that you create a weak persistent handle
using Dart_True for each of your isolates. Keep this weak persistent handle
around, so that you can pass it as the key. I am not sure, but it sort of sounds
like you are already doing something similar.

https://codereview.chromium.org/15772005/diff/24001/runtime/lib/mirrors.cc
File runtime/lib/mirrors.cc (right):

https://codereview.chromium.org/15772005/diff/24001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:147: ASSERT(!Dart_IsError(handle));
On 2013/05/28 17:37:18, siva wrote:
> ASSERT(handle != NULL);

Done.

https://codereview.chromium.org/15772005/diff/24001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/15772005/diff/24001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:203: Dart_Handle Api::Null(Isolate* isolate) {
On 2013/05/28 17:37:18, siva wrote:
> Is the isolate parameter still needed for these handle accessors?
> These handles are coming from the vm isolate.

Removed the isolate parameter as well as the unused pieces in ApiState.

[Anton Muhin, 2013-05-29 07:43:12]

Ivan, let's coordinate on launching.

https://codereview.chromium.org/15772005/diff/24001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/15772005/diff/24001/runtime/include/dart_api....
runtime/include/dart_api.h:473: Dart_WeakPersistentHandle* keys,
Yes, that's exactly what we do: weak persistent handle to always reachable
object.

On 2013/05/28 21:12:20, Ivan Posva wrote:
> On 2013/05/27 15:45:49, Anton Muhin wrote:
> > there was one fancy trick in our code which currently not applicable: we
need
> to
> > render some weak persistent handles as strong for GC.  We did it by creating
a
> > set with strong persistent handle.
> > 
> > I implemented trivial workaround for now: I create a weak persistent handle
> for
> > an object held by strong persistent handle, but if we can direct this case
in
> > API directly, that'd be great.
> > 
> > One simple approach would be either add a function or add a special
semantics
> > for references set with empty key sets.
> 
> I am not sure I understand your usage of this API function, but to me it
sounds
> like this:
> Occasionally you have a bunch of weak handles, which need to be preserved
during
> a GC. You used to create one strong persistent handle, use that as the "keys"
> and the to-be-preserved weak handles as the values. Do I understand that
> correctly?
> 
> If that is the case, then I suggest that you create a weak persistent handle
> using Dart_True for each of your isolates. Keep this weak persistent handle
> around, so that you can pass it as the key. I am not sure, but it sort of
sounds
> like you are already doing something similar.

[Ivan Posva, 2013-05-30 15:56:11]

Committed patchset #5 manually as r23421 (presubmit successful).