Improve C++ code for string splittig (Dromaeo benchmark).

Improve C++ code for string splittig (Dromaeo benchmark).
Committed: https://code.google.com/p/dart/source/detail?r=15710

[siva, 2012-12-04 03:04:50]

https://codereview.chromium.org/11308337/diff/4001/runtime/lib/string.cc
File runtime/lib/string.cc (right):

https://codereview.chromium.org/11308337/diff/4001/runtime/lib/string.cc#newc...
runtime/lib/string.cc:79: // 'isolate' is defined.
we have used isolate in the rest of the file with this assumption. Why special
case just this function.

https://codereview.chromium.org/11308337/diff/4001/runtime/lib/string.cc#newc...
runtime/lib/string.cc:97: isolate,
If we have to pass isolate as a parameter I would prefer
if it is the first argument.

https://codereview.chromium.org/11308337/diff/4001/runtime/lib/string.cc#newc...
runtime/lib/string.cc:99: result.Add(str, isolate);
Ditto comment.

https://codereview.chromium.org/11308337/diff/4001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/11308337/diff/4001/runtime/vm/object.cc#newco...
runtime/vm/object.cc:10742: Isolate* isolate,
Isolate is passed in here and not used at all, why pass it in?

https://codereview.chromium.org/11308337/diff/4001/runtime/vm/object.cc#newco...
runtime/vm/object.cc:11194: contents.SetAt(index, value);
Instead of passing in the isolate maybe we should consider changing the method
'SetAt' in GrowableObjectArray to do something like
StorePointer(ObjectAddr(index), value.raw());

Then we could call that directly here and not have the
handle creation here and in SetAt

[srdjan, 2012-12-04 21:41:30]

https://codereview.chromium.org/11308337/diff/4001/runtime/lib/string.cc
File runtime/lib/string.cc (right):

https://codereview.chromium.org/11308337/diff/4001/runtime/lib/string.cc#newc...
runtime/lib/string.cc:79: // 'isolate' is defined.
On 2012/12/04 03:04:50, siva wrote:
> we have used isolate in the rest of the file with this assumption. Why special
> case just this function.

Removed comment. I think it would be better if 'isolate' is renamed to make it
clear that it is coming from the macro.

https://codereview.chromium.org/11308337/diff/4001/runtime/lib/string.cc#newc...
runtime/lib/string.cc:97: isolate,
On 2012/12/04 03:04:50, siva wrote:
> If we have to pass isolate as a parameter I would prefer
> if it is the first argument.

Done.

https://codereview.chromium.org/11308337/diff/4001/runtime/lib/string.cc#newc...
runtime/lib/string.cc:99: result.Add(str, isolate);
On 2012/12/04 03:04:50, siva wrote:
> Ditto comment.

Done.

https://codereview.chromium.org/11308337/diff/4001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/11308337/diff/4001/runtime/vm/object.cc#newco...
runtime/vm/object.cc:10742: Isolate* isolate,
On 2012/12/04 03:04:50, siva wrote:
> Isolate is passed in here and not used at all, why pass it in?

Removed.

https://codereview.chromium.org/11308337/diff/4001/runtime/vm/object.cc#newco...
runtime/vm/object.cc:11194: contents.SetAt(index, value);
On 2012/12/04 03:04:50, siva wrote:
> Instead of passing in the isolate maybe we should consider changing the method
> 'SetAt' in GrowableObjectArray to do something like
> StorePointer(ObjectAddr(index), value.raw());
> 
> Then we could call that directly here and not have the
> handle creation here and in SetAt

Good idea. Discussed and Siva will take a look at it.

[siva, 2012-12-04 21:41:38]

LGTM (offline discussion to defer the SetAt change to
a new CL).