chakrashim: some performance improvements

IsTypeOf(): Replace built-in type check by name with by cached global
constructor function. This gets rid of repeated propertyId lookup and
getting property of global object.

Optimize likely int/double/bool conversions: In Value::Int32Value(), try
JsNumberToInt directly before performing JS value conversion, since the
underlying value is very likely an int already. Similarly for
Uint32Value(), BoolValue().

IsExternal(): Call JsHasProperty with a private symbol. Remove
IsObject() and propertyId lookup calls.

HandleScope: Use stack to save small number of refs. Skips slow JS array
code path for many scenarios.

Reviewed-by: @kunalspathak, @Cellule

deps/chakrashim/include/v8.h

@@ -422,11 +422,12 @@ class EXPORT Eternal : private Persistent<T> {
 // values created will then be added to that array. So the GC will see the array
 // on the stack and then keep those local references alive.
 class EXPORT HandleScope {
-  template <class T>
-  friend class Local;
+  template <class T> friend class Local;
+  static const int kOnStackLocals = 5;  // Arbitrary number of refs on stack
 
  private:
-  JsValueRef _refs;
+  JsValueRef _locals[kOnStackLocals];   // Save some refs on stack
+  JsValueRef _refs;                     // More refs go to a JS array
   int _count;
   HandleScope *_prev;
   JsContextRef _contextRef;

deps/chakrashim/src/jsrtcachedpropertyidref.inc

@@ -18,6 +18,14 @@
 // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
 // IN THE SOFTWARE.
 
+#ifndef DEF
+#define DEF(x)
+#endif
+
+#ifndef DEFSYMBOL
+#define DEFSYMBOL(x)
+#endif
+
 DEF(apply)
 DEF(construct)
 DEF(defineProperty)
@@ -63,4 +71,8 @@ DEF(buffer)
 DEF(byteOffset)
 DEF(byteLength)
 
+DEFSYMBOL(self)
+DEFSYMBOL(__isexternal__)
+
 #undef DEF
+#undef DEFSYMBOL

deps/chakrashim/src/jsrtcontextcachedobj.inc

@@ -30,14 +30,21 @@
 DEFTYPE(ArrayBuffer)
 DEFTYPE(Boolean)
 DEFTYPE(Date)
+DEFTYPE(Error)
+DEFTYPE(EvalError)
 DEFTYPE(Function)
 DEFTYPE(Number)
 DEFTYPE(Object)
 DEFTYPE(Proxy)
+DEFTYPE(RangeError)
+DEFTYPE(ReferenceError)
 DEFTYPE(RegExp)
 DEFTYPE(String)
-DEFTYPE(Uint8Array)
+DEFTYPE(SyntaxError)
+DEFTYPE(TypeError)
 DEFTYPE(Uint32Array)
+DEFTYPE(Uint8Array)
+DEFTYPE(URIError)
 
 
 // These prototype functions will be cached/shimmed

deps/chakrashim/src/jsrtisolateshim.cc

@@ -32,15 +32,15 @@ namespace jsrt {
 IsolateShim::IsolateShim(JsRuntimeHandle runtime)
     : runtime(runtime),
       contextScopeStack(nullptr),
-      selfSymbolPropertyIdRef(JS_INVALID_REFERENCE),
+      symbolPropertyIdRefs(),
       cachedPropertyIdRefs(),
+      embeddedData(),
       isDisposing(false),
       tryCatchStackTop(nullptr) {
   // CHAKRA-TODO: multithread locking for s_isolateList?
   this->prevnext = &s_isolateList;
   this->next = s_isolateList;
   s_isolateList = this;
-  memset(embeddedData, 0, sizeof(embeddedData));
 }
 
 IsolateShim::~IsolateShim() {
@@ -241,7 +241,7 @@ ContextShim * IsolateShim::GetCurrentContextShim() {
 }
 
 JsPropertyIdRef IsolateShim::GetSelfSymbolPropertyIdRef() {
-  return EnsurePrivateSymbol(&selfSymbolPropertyIdRef);
+  return GetCachedSymbolPropertyIdRef(CachedSymbolPropertyIdRef::self);
 }
 
 JsPropertyIdRef IsolateShim::GetKeepAliveObjectSymbolPropertyIdRef() {
@@ -250,20 +250,29 @@ JsPropertyIdRef IsolateShim::GetKeepAliveObjectSymbolPropertyIdRef() {
   // return EnsurePrivateSymbol(&keepAliveObjectSymbolPropertyIdRef);
 }
 
-JsPropertyIdRef IsolateShim::EnsurePrivateSymbol(
-    JsPropertyIdRef * propertyIdRefPtr) {
-  assert(this->GetCurrentContextShim() != nullptr);
-  JsPropertyIdRef propertyIdRef = *propertyIdRefPtr;
-  if (propertyIdRef == JS_INVALID_REFERENCE) {
-    JsValueRef newSymbol;
-    if (JsCreateSymbol(JS_INVALID_REFERENCE, &newSymbol) == JsNoError) {
-      if (JsGetPropertyIdFromSymbol(newSymbol, &propertyIdRef) == JsNoError) {
-        JsAddRef(propertyIdRef, nullptr);
-        *propertyIdRefPtr = propertyIdRef;
-      }
+template <class Index, class CreatePropertyIdFunc>
+JsPropertyIdRef GetCachedPropertyId(
+    JsPropertyIdRef cache[], Index index,
+    const CreatePropertyIdFunc& createPropertyId) {
+  JsPropertyIdRef propIdRef = cache[index];
+  if (propIdRef == JS_INVALID_REFERENCE) {
+    if (createPropertyId(index, &propIdRef)) {
+      JsAddRef(propIdRef, nullptr);
+      cache[index] = propIdRef;
     }
   }
-  return propertyIdRef;
+  return propIdRef;
+}
+
+JsPropertyIdRef IsolateShim::GetCachedSymbolPropertyIdRef(
+    CachedSymbolPropertyIdRef cachedSymbolPropertyIdRef) {
+  CHAKRA_ASSERT(this->GetCurrentContextShim() != nullptr);
+  return GetCachedPropertyId(symbolPropertyIdRefs, cachedSymbolPropertyIdRef,
+                    [](CachedSymbolPropertyIdRef, JsPropertyIdRef* propIdRef) {
+      JsValueRef newSymbol;
+      return JsCreateSymbol(JS_INVALID_REFERENCE, &newSymbol) == JsNoError &&
+        JsGetPropertyIdFromSymbol(newSymbol, propIdRef) == JsNoError;
+  });
 }
 
 static wchar_t const *
@@ -274,15 +283,11 @@ const s_cachedPropertyIdRefNames[CachedPropertyIdRef::Count] = {
 
 JsPropertyIdRef IsolateShim::GetCachedPropertyIdRef(
     CachedPropertyIdRef cachedPropertyIdRef) {
-  JsPropertyIdRef propertyIdRef = cachedPropertyIdRefs[cachedPropertyIdRef];
-  if (propertyIdRef == JS_INVALID_REFERENCE) {
-    if (JsGetPropertyIdFromName(s_cachedPropertyIdRefNames[cachedPropertyIdRef],
-                                &propertyIdRef) == JsNoError) {
-      JsAddRef(propertyIdRef, nullptr);
-      cachedPropertyIdRefs[cachedPropertyIdRef] = propertyIdRef;
-    }
-  }
-  return propertyIdRef;
+  return GetCachedPropertyId(cachedPropertyIdRefs, cachedPropertyIdRef,
+                    [](CachedPropertyIdRef index, JsPropertyIdRef* propIdRef) {
+    return JsGetPropertyIdFromName(s_cachedPropertyIdRefNames[index],
+                                   propIdRef) == JsNoError;
+  });
 }
 
 JsPropertyIdRef IsolateShim::GetProxyTrapPropertyIdRef(ProxyTraps trap) {

deps/chakrashim/src/jsrtisolateshim.h

@@ -36,6 +36,12 @@ enum CachedPropertyIdRef {
   Count
 };
 
+enum CachedSymbolPropertyIdRef {
+#define DEFSYMBOL(x, ...) x,
+#include "jsrtcachedpropertyidref.inc"
+  SymbolCount
+};
+
 class IsolateShim {
  public:
 
@@ -67,6 +73,8 @@ class IsolateShim {
   // Symbols propertyIdRef
   JsPropertyIdRef GetSelfSymbolPropertyIdRef();
   JsPropertyIdRef GetKeepAliveObjectSymbolPropertyIdRef();
+  JsPropertyIdRef GetCachedSymbolPropertyIdRef(
+    CachedSymbolPropertyIdRef cachedSymbolPropertyIdRef);
 
   // String propertyIdRef
   JsPropertyIdRef GetProxyTrapPropertyIdRef(ProxyTraps trap);
@@ -97,10 +105,8 @@ class IsolateShim {
   static void CALLBACK JsContextBeforeCollectCallback(
     _In_ JsRef contextRef, _In_opt_ void *data);
 
-  JsPropertyIdRef EnsurePrivateSymbol(JsPropertyIdRef * propertyIdRefPtr);
-
   JsRuntimeHandle runtime;
-  JsPropertyIdRef selfSymbolPropertyIdRef;
+  JsPropertyIdRef symbolPropertyIdRefs[CachedSymbolPropertyIdRef::SymbolCount];
   JsPropertyIdRef cachedPropertyIdRefs[CachedPropertyIdRef::Count];
   bool isDisposing;
 

deps/chakrashim/src/jsrtutils.cc

@@ -76,12 +76,7 @@ JsErrorCode GetProperty(_In_ JsValueRef ref,
     return error;
   }
 
-  error = JsConvertValueToNumber(value, &value);
-  if (error != JsNoError) {
-    return error;
-  }
-
-  return JsNumberToInt(value, intValue);
+  return ValueToIntLikely(value, intValue);
 }
 
 JsErrorCode SetProperty(_In_ JsValueRef ref,
@@ -200,12 +195,7 @@ JsErrorCode CallGetter(
     return error;
   }
 
-  error = JsConvertValueToNumber(value, &value);
-  if (error != JsNoError) {
-    return error;
-  }
-
-  return JsNumberToInt(value, result);
+  return ValueToIntLikely(value, result);
 }
 
 JsErrorCode GetPropertyOfGlobal(_In_ const wchar_t *propertyName,

deps/chakrashim/src/jsrtutils.h

@@ -363,4 +363,50 @@ class JsArguments {
 // create and set the exception on the context
 void SetOutOfMemoryErrorIfExist(_In_ JsErrorCode errorCode);
 
+
+template <bool LIKELY,
+          class JsConvertToValueFunc,
+          class JsValueToNativeFunc,
+          class T>
+JsErrorCode ValueToNative(const JsConvertToValueFunc& JsConvertToValue,
+                          const JsValueToNativeFunc& JsValueToNative,
+                          JsValueRef value, T* nativeValue) {
+  JsErrorCode error;
+
+  // If LIKELY, try JsValueToNative first. Likely to succeed.
+  if (LIKELY) {
+    error = JsValueToNative(value, nativeValue);
+    if (error != JsErrorInvalidArgument) {
+      return error;
+    }
+  }
+
+  // Perform JS conversion first, then to native.
+  error = JsConvertToValue(value, &value);
+  if (error != JsNoError) {
+    return error;
+  }
+  return JsValueToNative(value, nativeValue);
+}
+
+inline JsErrorCode ValueToInt(JsValueRef value, int* intValue) {
+  return ValueToNative</*LIKELY*/false>(
+    JsConvertValueToNumber, JsNumberToInt, value, intValue);
+}
+
+inline JsErrorCode ValueToIntLikely(JsValueRef value, int* intValue) {
+  return ValueToNative</*LIKELY*/true>(
+    JsConvertValueToNumber, JsNumberToInt, value, intValue);
+}
+
+inline JsErrorCode ValueToDouble(JsValueRef value, double* dblValue) {
+  return ValueToNative</*LIKELY*/false>(
+    JsConvertValueToNumber, JsNumberToDouble, value, dblValue);
+}
+
+inline JsErrorCode ValueToDoubleLikely(JsValueRef value, double* dblValue) {
+  return ValueToNative</*LIKELY*/true>(
+    JsConvertValueToNumber, JsNumberToDouble, value, dblValue);
+}
+
 }  // namespace jsrt

deps/chakrashim/src/v8external.cc

@@ -25,8 +25,6 @@ namespace v8 {
 
 using jsrt::IsolateShim;
 
-const wchar_t * EXTERNAL_PROP_NAME = L"__isexternal__";
-
 Local<Value> External::Wrap(void* data) {
   return External::New(Isolate::GetCurrent(), data);
 }
@@ -46,14 +44,15 @@ Local<External> External::New(Isolate* isolate, void* value) {
     return Local<External>();
   }
 
-  JsValueRef trueRef =
-    IsolateShim::FromIsolate(isolate)->GetCurrentContextShim()->GetTrue();
+  IsolateShim* iso = IsolateShim::FromIsolate(isolate);
+  JsValueRef trueRef = iso->GetCurrentContextShim()->GetTrue();
   if (JsGetTrueValue(&trueRef) != JsNoError) {
     return Local<External>();
   }
 
   if (jsrt::DefineProperty(externalRef,
-                           EXTERNAL_PROP_NAME,
+                           iso->GetCachedSymbolPropertyIdRef(
+                             jsrt::CachedSymbolPropertyIdRef::__isexternal__),
                            jsrt::PropertyDescriptorOptionValues::False,
                            jsrt::PropertyDescriptorOptionValues::False,
                            jsrt::PropertyDescriptorOptionValues::False,
@@ -67,17 +66,13 @@ Local<External> External::New(Isolate* isolate, void* value) {
 }
 
 bool External::IsExternal(const v8::Value* value) {
-  if (!value->IsObject()) {
-    return false;
-  }
-
-  JsPropertyIdRef propIdRef;
-  if (JsGetPropertyIdFromName(EXTERNAL_PROP_NAME, &propIdRef) != JsNoError) {
-    return false;
-  }
+  IsolateShim* iso = IsolateShim::GetCurrent();
 
   bool hasProp;
-  if (JsHasProperty((JsValueRef)value, propIdRef, &hasProp) != JsNoError) {
+  if (JsHasProperty((JsValueRef)value,
+                    iso->GetCachedSymbolPropertyIdRef(
+                      jsrt::CachedSymbolPropertyIdRef::__isexternal__),
+                    &hasProp) != JsNoError) {
     return false;
   }
 

deps/chakrashim/src/v8handlescope.cc

@@ -27,6 +27,7 @@ __declspec(thread) HandleScope *current = nullptr;
 
 HandleScope::HandleScope(Isolate* isolate)
     : _prev(current),
+      _locals(),
       _refs(JS_INVALID_REFERENCE),
       _count(0),
       _contextRef(JS_INVALID_REFERENCE),
@@ -52,6 +53,11 @@ HandleScope *HandleScope::GetCurrent() {
 }
 
 bool HandleScope::AddLocal(JsValueRef value) {
+  if (_count < _countof(_locals)) {
+    _locals[_count++] = value;
+    return true;
+  }
+
   if (_refs == JS_INVALID_REFERENCE) {
     if (JsCreateArray(1, &_refs) != JsNoError) {
       return AddLocalAddRef(value);
@@ -60,7 +66,7 @@ bool HandleScope::AddLocal(JsValueRef value) {
 
   JsValueRef index;
 
-  if (JsIntToNumber(_count, &index) != JsNoError) {
+  if (JsIntToNumber(_count - _countof(_locals), &index) != JsNoError) {
     return AddLocalAddRef(value);
   }