Implement external strings.

Implement external strings.

External strings refer to character data in memory located outside of
the managed heap.  This memory is considered to be owned by the
embedding application.  To help with the management of external
memory, the virtual machine allows the embedding application to
register for a "death notice" when an external string object is
garbage collected.  A death notice takes the form of a callback
invoked by the garbage collector with the address of the external
memory.  Death notices will be implemented in the garbage collector by
future commit.

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

[Anton Muhin, 2011-10-25 10:19:02]

DBC

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h#n...
runtime/include/dart_api.h:300: DART_EXPORT Dart_Handle Dart_NewExternalString8(
this API might be difficult to use by embedder.  For example, WebCore strings
are refcounted objects and it's difficult (and in some cases impossible) to find
a String object out of content chars pointer.  May we have a second void*
pointer to be passed back into finalizer callback, please?

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h#n...
runtime/include/dart_api.h:304: DART_EXPORT Dart_Handle
Dart_NewExternalString16(
Another note, just in case: ideally we need a way to turn DartVM string into an
externalized one.

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/raw_object_snapsh...
File runtime/vm/raw_object_snapshot.cc (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/raw_object_snapsh...
runtime/vm/raw_object_snapshot.cc:1137: RawExternalOneByteString*
ExternalOneByteString::ReadFrom(
should we allow to serialize externalized strings?  it might be difficult to
reattach a finalizer callback.

[turnidge, 2011-10-25 18:07:20]

DBC -turnidge

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:789: intptr_t length) {
You have updated the Dart_isString* functions but not the Dart_StringGet*
functions.  Is that intentional?

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:5614: // Once we have external string support, this will
also create a heap copy of
Update this comment?

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/object.h
File runtime/vm/object.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/object.h#newcode2378
runtime/vm/object.h:2378: }
Passing thought... Would it make sense to add a virtual to the base string that
lets you just get the character size directly?  Something like
'GetCharacterSize()' that returns 1, 2, or 4?

It would let you use "int size = max(s1.GetCharacterSize(),
s2.GetCharacterSize())" inside of Concat...

Feel free to file this comment for later...

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:696: uint8_t* data_;
Out of curiosity, can we rely on external strings being null terminated?

[cshapiro, 2011-10-25 20:42:47]

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h#n...
runtime/include/dart_api.h:300: DART_EXPORT Dart_Handle Dart_NewExternalString8(
Sure, I'll add and additional argument.

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h#n...
runtime/include/dart_api.h:304: DART_EXPORT Dart_Handle
Dart_NewExternalString16(
Doesn't the embedding API provide a way to do this by way of the
Dart_StringGet8, Dart_StringGet16 and Dart_StringGet32?

Can you give me an example of when you would need this?

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:789: intptr_t length) {
Nope, that was an oversight..

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:5614: // Once we have external string support, this will
also create a heap copy of
Yes, indeed.  Thanks for pointing that out.

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/object.h
File runtime/vm/object.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/object.h#newcode2378
runtime/vm/object.h:2378: }
Sounds like a good idea.  This will show up in my next patch set.

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:696: uint8_t* data_;
Certainly not.  We inherit a length field from the String class which makes that
kind of length encoding unnecessary.

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/raw_object_snapsh...
File runtime/vm/raw_object_snapshot.cc (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/raw_object_snapsh...
runtime/vm/raw_object_snapshot.cc:1137: RawExternalOneByteString*
ExternalOneByteString::ReadFrom(
No, we should not.  What this code does is serialize an external string as an
ordinary dart string of the equivalent character width.  This prevents external
strings from appearing in the snapshot.

[turnidge, 2011-10-25 20:45:15]

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:696: uint8_t* data_;
On 2011/10/25 20:42:47, cshapiro wrote:
> Certainly not.  We inherit a length field from the String class which makes
that
> kind of length encoding unnecessary.

Ah, thanks.

[Anton Muhin, 2011-10-26 14:11:52]

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h#n...
runtime/include/dart_api.h:304: DART_EXPORT Dart_Handle
Dart_NewExternalString16(
Not quite.  I do not want to fetch a codepoints out of a DartVM string, but
rather turn a DartVM string into externalized one, so later I can perform this
conversion faster.

Use case: we want to get maximum speed out of calls like foo("bar") where "bar"
needs to be converted into embedder's string.  "bar" is DartVM symbol and w/o
ability to turn it into externalized string, embedder will have to convert it on
each invocation.  If embedder can externalize "bar", on subsequent conversions,
(s)he can fetch embedder's sight representation out of "bar" immediately (peer
in your parlance).

v8 achieves it be overwriting string object in the heap.  It's not always
possible (0 length string or even longer one on x64), but works pretty nice.

On 2011/10/25 20:42:47, cshapiro wrote:
> Doesn't the embedding API provide a way to do this by way of the
> Dart_StringGet8, Dart_StringGet16 and Dart_StringGet32?
> 
> Can you give me an example of when you would need this?

http://codereview.chromium.org/8383029/diff/7002/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/8383029/diff/7002/runtime/include/dart_api.h#n...
runtime/include/dart_api.h:101: typedef void
(*Dart_ExternalString8Finalizer)(uint8_t* codepoints,
I think codepoints may go away---embedder should be able to restore codepoints
from peer if need be.

[cshapiro, 2011-10-26 23:35:08]

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h#n...
runtime/include/dart_api.h:304: DART_EXPORT Dart_Handle
Dart_NewExternalString16(
Okay, I think I understand more about what you want...

Obviously, we are doing this a little bit differently from V8.  V8 requires the
embedding application to provide a C++ instance which implement an external
string interface.  This gives the embedder a lot of flexibility as all manner of
functionality can be buried in the class instance.

This API does something different.  We ask for only the data and length at
construction time.  However, we can do something similar to V8.  Instead of
passing a data pointer, we could take 2 arguments at construction time

  1) a vtable-like C structure with function pointers for each logical interface
method
  2) an otherwise opaque instance pointer

To externalize a string, we could require the vtable in 1 to also include a
method to construct an instance of the embedder's choosing and then the VM would
arrange to change the type of the string instance.

It still would be nice, from the perspective of performance, to be able to cache
both the length and data fields in the header of the external string object.  In
your other recent review comment you suggested the data field may not always be
valid.  I would like to understand more about situations when that could happen
to ensure we have the best design.  Could you clarify?

[Anton Muhin, 2011-10-27 14:14:46]

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h#n...
runtime/include/dart_api.h:304: DART_EXPORT Dart_Handle
Dart_NewExternalString16(
Sorry, I didn't mean data is invalid.  I meant it difficult to find a real owner
of data out of data.

Sorry, I am not sure I remember saying data maybe invalid.  I believe that data
must be immutable and data should be always valid.

Regarding the caching: the only concern is it may make externalized string
object bigger hence making impossible to turn short VM string into an external
one.

On 2011/10/26 23:35:08, cshapiro wrote:
> Okay, I think I understand more about what you want...
> 
> Obviously, we are doing this a little bit differently from V8.  V8 requires
the
> embedding application to provide a C++ instance which implement an external
> string interface.  This gives the embedder a lot of flexibility as all manner
of
> functionality can be buried in the class instance.
> 
> This API does something different.  We ask for only the data and length at
> construction time.  However, we can do something similar to V8.  Instead of
> passing a data pointer, we could take 2 arguments at construction time
> 
>   1) a vtable-like C structure with function pointers for each logical
interface
> method
>   2) an otherwise opaque instance pointer
> 
> To externalize a string, we could require the vtable in 1 to also include a
> method to construct an instance of the embedder's choosing and then the VM
would
> arrange to change the type of the string instance.
> 
> It still would be nice, from the perspective of performance, to be able to
cache
> both the length and data fields in the header of the external string object. 
In
> your other recent review comment you suggested the data field may not always
be
> valid.  I would like to understand more about situations when that could
happen
> to ensure we have the best design.  Could you clarify?

[cshapiro, 2011-10-27 21:12:51]

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h#n...
runtime/include/dart_api.h:304: DART_EXPORT Dart_Handle
Dart_NewExternalString16(
Thanks for clarifying.  I think we can do what you want although I would like to
have an understanding of what the minimum object size must be.  It is not hard
to imagine cases where large strings benefit from being externalized.  However,
I am wondering how often this is true for shorter strings.

Can we run an experiment to see how often strings of specific lengths are
externalized?  A histogram would be helpful to understand the break-even point.

As an aside, I am not worried about space, even in small strings.  We can always
add an additional indirection to storage outside the object to allows small
strings can be externalized.  At the moment, I am most concerned about getting
the API just right so important use cases are easily accommodated.

[Anton Muhin, 2011-10-31 09:02:03]

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h#n...
runtime/include/dart_api.h:304: DART_EXPORT Dart_Handle
Dart_NewExternalString16(
I'll run GMail in customized build to gather the stats today/tomorrow.  I would
expect a lot of tiny string as size of the source code in JS is very important
and devs do their best to minimize it.

On 2011/10/27 21:12:51, cshapiro wrote:
> Thanks for clarifying.  I think we can do what you want although I would like
to
> have an understanding of what the minimum object size must be.  It is not hard
> to imagine cases where large strings benefit from being externalized. 
However,
> I am wondering how often this is true for shorter strings.
> 
> Can we run an experiment to see how often strings of specific lengths are
> externalized?  A histogram would be helpful to understand the break-even
point.
> 
> As an aside, I am not worried about space, even in small strings.  We can
always
> add an additional indirection to storage outside the object to allows small
> strings can be externalized.  At the moment, I am most concerned about getting
> the API just right so important use cases are easily accommodated.

[Anton Muhin, 2011-11-01 17:40:44]

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/8383029/diff/2001/runtime/include/dart_api.h#n...
runtime/include/dart_api.h:304: DART_EXPORT Dart_Handle
Dart_NewExternalString16(
Promised data (I collected all the string which can get externalized, not
actually will).  The data after ~1min of corp GMail:

    0:     667 (10.0%)
    1:      17 ( 0.3%)
    2:     293 ( 4.4%)
    3:    2175 (32.7%)
    4:     119 ( 1.8%)
    5:     458 ( 6.9%)
    6:     518 ( 7.8%)
    7:     381 ( 5.7%)
    8:      72 ( 1.1%)
    9:     128 ( 1.9%)
   10:     368 ( 5.5%)
   11:     130 ( 2.0%)
   12:     162 ( 2.4%)
   13:     308 ( 4.6%)
   14:      19 ( 0.3%)
   15:      51 ( 0.8%)
   16:      47 ( 0.7%)
   17:      11 ( 0.2%)
   18:      19 ( 0.3%)
   19:      41 ( 0.6%)
   20:      55 ( 0.8%)
   21:      11 ( 0.2%)
   22:      13 ( 0.2%)
   23:      10 ( 0.2%)
   24:       3 ( 0.0%)
   25:       3 ( 0.0%)
   26:      18 ( 0.3%)
   27:      20 ( 0.3%)
   28:       5 ( 0.1%)
   29:      11 ( 0.2%)
   30:       0 ( 0.0%)
   31:      27 ( 0.4%)
  32+:     496 ( 7.5%)
total:    6656 (100.0%)

On the earlier stages 3chars strings are even more visible:

    0:       0 ( 0.0%)
    1:       8 ( 3.1%)
    2:      54 (21.1%)
    3:     162 (63.3%)
    4:       2 ( 0.8%)
    5:       2 ( 0.8%)
    6:       1 ( 0.4%)
    7:       3 ( 1.2%)
    8:       1 ( 0.4%)
    9:       2 ( 0.8%)
   10:       1 ( 0.4%)
   11:       3 ( 1.2%)
   12:       4 ( 1.6%)
   13:       5 ( 2.0%)
   14:       1 ( 0.4%)
   15:       0 ( 0.0%)
   16:       1 ( 0.4%)
   17:       1 ( 0.4%)
   18:       1 ( 0.4%)
   19:       0 ( 0.0%)
   20:       0 ( 0.0%)
   21:       0 ( 0.0%)
   22:       0 ( 0.0%)
   23:       0 ( 0.0%)
   24:       0 ( 0.0%)
   25:       0 ( 0.0%)
   26:       0 ( 0.0%)
   27:       0 ( 0.0%)
   28:       0 ( 0.0%)
   29:       0 ( 0.0%)
   30:       0 ( 0.0%)
   31:       0 ( 0.0%)
  32+:       4 ( 1.6%)

total:     256 (100.0%)
    0:       1 ( 0.2%)
    1:       8 ( 1.6%)
    2:     102 (19.9%)
    3:     359 (70.1%)
    4:       2 ( 0.4%)
    5:       2 ( 0.4%)
    6:       2 ( 0.4%)
    7:       4 ( 0.8%)
    8:       2 ( 0.4%)
    9:       2 ( 0.4%)
   10:       1 ( 0.2%)
   11:       4 ( 0.8%)
   12:       4 ( 0.8%)
   13:       5 ( 1.0%)
   14:       2 ( 0.4%)
   15:       1 ( 0.2%)
   16:       2 ( 0.4%)
   17:       1 ( 0.2%)
   18:       2 ( 0.4%)
   19:       1 ( 0.2%)
   20:       0 ( 0.0%)
   21:       0 ( 0.0%)
   22:       0 ( 0.0%)
   23:       0 ( 0.0%)
   24:       0 ( 0.0%)
   25:       0 ( 0.0%)
   26:       0 ( 0.0%)
   27:       0 ( 0.0%)
   28:       0 ( 0.0%)
   29:       0 ( 0.0%)
   30:       0 ( 0.0%)
   31:       0 ( 0.0%)
  32+:       5 ( 1.0%)
total:     512 (100.0%)


    0:       1 ( 0.1%)
    1:       8 ( 0.8%)
    2:     103 (10.1%)
    3:     870 (85.0%)
    4:       2 ( 0.2%)
    5:       2 ( 0.2%)
    6:       2 ( 0.2%)
    7:       4 ( 0.4%)
    8:       2 ( 0.2%)
    9:       2 ( 0.2%)
   10:       1 ( 0.1%)
   11:       4 ( 0.4%)
   12:       4 ( 0.4%)
   13:       5 ( 0.5%)
   14:       2 ( 0.2%)
   15:       1 ( 0.1%)
   16:       2 ( 0.2%)
   17:       1 ( 0.1%)
   18:       2 ( 0.2%)
   19:       1 ( 0.1%)
   20:       0 ( 0.0%)
   21:       0 ( 0.0%)
   22:       0 ( 0.0%)
   23:       0 ( 0.0%)
   24:       0 ( 0.0%)
   25:       0 ( 0.0%)
   26:       0 ( 0.0%)
   27:       0 ( 0.0%)
   28:       0 ( 0.0%)
   29:       0 ( 0.0%)
   30:       0 ( 0.0%)
   31:       0 ( 0.0%)
  32+:       5 ( 0.5%)
total:    1024 (100.0%)

I would speculate that most of 3 chars strings are id/classes.  So it looks like
ideally we should be able to externalize strings of 3+ length.

WDYT?


On 2011/10/31 09:02:04, antonmuhin wrote:
> I'll run GMail in customized build to gather the stats today/tomorrow.  I
would
> expect a lot of tiny string as size of the source code in JS is very important
> and devs do their best to minimize it.
> 
> On 2011/10/27 21:12:51, cshapiro wrote:
> > Thanks for clarifying.  I think we can do what you want although I would
like
> to
> > have an understanding of what the minimum object size must be.  It is not
hard
> > to imagine cases where large strings benefit from being externalized. 
> However,
> > I am wondering how often this is true for shorter strings.
> > 
> > Can we run an experiment to see how often strings of specific lengths are
> > externalized?  A histogram would be helpful to understand the break-even
> point.
> > 
> > As an aside, I am not worried about space, even in small strings.  We can
> always
> > add an additional indirection to storage outside the object to allows small
> > strings can be externalized.  At the moment, I am most concerned about
getting
> > the API just right so important use cases are easily accommodated.
>

[cshapiro, 2011-11-01 18:36:06]

Hi Anton,

Thanks for collecting this data.  It is very useful.  Do the counts on the left
hand side of your tables represent unique strings instances or any string
instance observed by an API call?

I have a plan to reorganize the way instances are formatted to allow strings of
0 or more characters to be externalized similar to the way V8 does it.  I am a
little worried as externalizing a small string will have considerable space
overhead, but this is already the case in V8.  So, unless we can measure time
saving, we may want to threshold the externalize requests so small strings are
left as-is.

Carl

[cshapiro, 2011-11-10 00:24:06]

PTAL

[cshapiro, 2011-11-10 00:24:07]

PTAL

[Anton Muhin, 2011-11-10 09:12:01]

I think we can do it in iterations.  I'd suggest that you implement it in the
best way from VM point of view.  Then later we'll start a serious perf work on
this stuff, we may reconsider some of current design choices.  So if possible,
be ready. 

Anyway, I don't think this particular CL should be blocked due to new feature
request.

On 2011/11/01 18:36:06, cshapiro wrote:
> Hi Anton,
> 
> Thanks for collecting this data.  It is very useful.  Do the counts on the
left
> hand side of your tables represent unique strings instances or any string
> instance observed by an API call?
> 
> I have a plan to reorganize the way instances are formatted to allow strings
of
> 0 or more characters to be externalized similar to the way V8 does it.  I am a
> little worried as externalizing a small string will have considerable space
> overhead, but this is already the case in V8.  So, unless we can measure time
> saving, we may want to threshold the externalize requests so small strings are
> left as-is.
> 
> Carl

[Ivan Posva, 2011-11-18 19:36:43]

Please rename the external string data type and add a test that verifies the
Dart_IsString16 implementation and then LGTM.

-Ivan

http://codereview.chromium.org/8383029/diff/16001/runtime/lib/string.dart
File runtime/lib/string.dart (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/lib/string.dart#new...
runtime/lib/string.dart:422: bool _isWhitespace(int codePoint) {
Shouldn't this be factored out into a private static method shared by all String
implementations? Different CL though, filed bug:
http://code.google.com/p/dart/issues/detail?id=525

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:843: obj.IsTwoByteString() ||
obj.IsExternalOneByteString());
obj.IsExternalOneByteString() -> obj.IsExternalTwoByteString()

Missing test?

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object.cc#newcod...
runtime/vm/object.cc:6566: RawExternalStringData<uint8_t>* external_data =
Naming this Raw is very confusing, the expectation is that this is allocated in
the heap, which it obviously isn't.

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object_store.cc
File runtime/vm/object_store.cc (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object_store.cc#...
runtime/vm/object_store.cc:27: four_byte_string_class_(Class::null()),
New fields are not being initialized.

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/raw_object.h#new...
runtime/vm/raw_object.h:692: class RawExternalStringData {
As I mentioned before this should really not be named Raw.

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/raw_object_snaps...
File runtime/vm/raw_object_snapshot.cc (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/raw_object_snaps...
runtime/vm/raw_object_snapshot.cc:1144: void
RawExternalOneByteString::WriteTo(SnapshotWriter* writer,
Maybe a comment stating that external strings are converted to non-external when
serialized? Here and two places below.

[siva, 2011-11-18 23:26:09]

lgtm

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:799: HandleScope scope;  // Setup a VM handle scope.
When you sync up these two lines will be replaced by
DART_SCOPE(Isolate::Current());

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:854: String& string_obj = String::Handle();
If you manage to get your change in before Todd then you are ok :-) otherwise
there is a synch up here too, I beleive he has a Api::UnwrapStringHandle(..)
method.

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object.h
File runtime/vm/object.h (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object.h#newcode...
runtime/vm/object.h:2381: typedef void (*PeerFinalizer)(void *peer);
dart_api.h is included in this file so can we use the same typedef that is
already defined in that file?

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object.h#newcode...
runtime/vm/object.h:2706: return &raw_ptr()->external_data_->data_[index];
I normally put a paren around as follows:
return &(raw_ptr()->external_data_->data_[index]);
to be more explicit but feel free to ignore this comment.

[cshapiro, 2011-11-19 01:08:29]

http://codereview.chromium.org/8383029/diff/16001/runtime/lib/string.dart
File runtime/lib/string.dart (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/lib/string.dart#new...
runtime/lib/string.dart:422: bool _isWhitespace(int codePoint) {
Yes.  My intention was to replace all of these methods with a native call to a
character property predicate that checks to see if the character has the Unicode
"White_Space" property.

At present, there are 26 code points with this property.  

http://www.unicode.org/Public/UNIDATA/PropList.txt

Because the cardinality of this set is low, a switch statement in Dart might be
the fastest way to implement the membership test.

I think we will want this functionality in native code anyway for the benefit of
the scanner.  We can mirror it in Dart if it turns out to be a hot-spot.

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:799: HandleScope scope;  // Setup a VM handle scope.
Thanks for pointing this out.  I discovered the change after I resolved the
merge conflicts and tried to build for the first time.

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:843: obj.IsTwoByteString() ||
obj.IsExternalOneByteString());
Fixed.  I could not find any tests for this function.  I'll put something
together in a follow-up change.

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object.cc#newcod...
runtime/vm/object.cc:6566: RawExternalStringData<uint8_t>* external_data =
Fixed.

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object.h
File runtime/vm/object.h (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object.h#newcode...
runtime/vm/object.h:2381: typedef void (*PeerFinalizer)(void *peer);
Good idea.  I am thinking of getting rid of this typedef when I provide
finalization for ExternalArrays.  I will delete this constant and use the Dart
API one when that happens.

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object.h#newcode...
runtime/vm/object.h:2706: return &raw_ptr()->external_data_->data_[index];
Sure, why not?  Fixed here and in the two cases below as well.

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object_store.cc
File runtime/vm/object_store.cc (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/object_store.cc#...
runtime/vm/object_store.cc:27: four_byte_string_class_(Class::null()),
Done.  I also added an initialization for the new byte buffer class.

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/raw_object.h#new...
runtime/vm/raw_object.h:692: class RawExternalStringData {
Done.  This has been renamed to ExternalStringData.

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/raw_object_snaps...
File runtime/vm/raw_object_snapshot.cc (right):

http://codereview.chromium.org/8383029/diff/16001/runtime/vm/raw_object_snaps...
runtime/vm/raw_object_snapshot.cc:1144: void
RawExternalOneByteString::WriteTo(SnapshotWriter* writer,
Certainly.  Done.