Add fast paths for Smi values in the dart embedding api.

Add fast paths for Smi values in the dart embedding api.

This speeds up the UseDartApi microbenchmark by 30%.  Thanks to Carl
for suggesting this.
Committed: https://code.google.com/p/dart/source/detail?r=6602

[cshapiro, 2012-04-11 23:29:09]

lgtm

if possible reusing the equivalent code in the Smi or RawObject types is
preferable.

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/dart_api_impl.h
File vm/dart_api_impl.h (right):

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/dart_api_impl.h#...
vm/dart_api_impl.h:115: static bool IsSmi(Dart_Handle handle) {
Why not call the existing predicate after casing the value to a RawObject?

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/dart_api_impl.h#...
vm/dart_api_impl.h:122: intptr_t value = *(reinterpret_cast<intptr_t*>(handle));
This may be another opportunity to reuse existing infrastructure.

[siva, 2012-04-12 02:18:13]

DBC

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/dart_api_impl.cc
File vm/dart_api_impl.cc (right):

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/dart_api_impl.cc...
vm/dart_api_impl.cc:1166: if ((value <= Smi::kMaxValue) && (value >=
Smi::kMinValue)) {
if (Smi::IsValid64(value)) {

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/handles.h
File vm/handles.h (right):

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/handles.h#newcod...
vm/handles.h:313: #endif  // defined(DEBUG)
Does this make a big difference in the numbers?

Sometimes it feels more readable not having a macro but
instead declaring an object:
{
  NoHandleScope no_handles;

  ...
  ...
}

Not a big deal, but if the performance difference is not significant than having
that option would be good.

[turnidge, 2012-04-16 20:07:24]

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/dart_api_impl.cc
File vm/dart_api_impl.cc (right):

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/dart_api_impl.cc...
vm/dart_api_impl.cc:1166: if ((value <= Smi::kMaxValue) && (value >=
Smi::kMinValue)) {
On 2012/04/12 02:18:13, asiva wrote:
> if (Smi::IsValid64(value)) {

Done.

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/dart_api_impl.h
File vm/dart_api_impl.h (right):

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/dart_api_impl.h#...
vm/dart_api_impl.h:115: static bool IsSmi(Dart_Handle handle) {
On 2012/04/11 23:29:09, cshapiro wrote:
> Why not call the existing predicate after casing the value to a RawObject?

Done.

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/dart_api_impl.h#...
vm/dart_api_impl.h:122: intptr_t value = *(reinterpret_cast<intptr_t*>(handle));
On 2012/04/11 23:29:09, cshapiro wrote:
> This may be another opportunity to reuse existing infrastructure.

Done.  Had to add friend permission though...

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/handles.h
File vm/handles.h (right):

https://chromiumcodereview.appspot.com/10052027/diff/6001/vm/handles.h#newcod...
vm/handles.h:313: #endif  // defined(DEBUG)
I had seen a difference before but was unable to reproduce it when I tried to
today.  I have reverted the change to handles.h, etc.

On 2012/04/12 02:18:13, asiva wrote:
> Does this make a big difference in the numbers?
> 
> Sometimes it feels more readable not having a macro but
> instead declaring an object:
> {
>   NoHandleScope no_handles;
> 
>   ...
>   ...
> }
> 
> Not a big deal, but if the performance difference is not significant than
having
> that option would be good.

[cshapiro, 2012-04-16 20:32:45]

https://chromiumcodereview.appspot.com/10052027/diff/13001/vm/object.h
File vm/object.h (right):

https://chromiumcodereview.appspot.com/10052027/diff/13001/vm/object.h#newcod...
vm/object.h:2750: friend class Api;  // For ValueFromRaw
Co-locate this with the other friend classes below?