Changeset 241956 in webkit

Timestamp:

Feb 22, 2019 12:20:49 PM (5 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] putNonEnumerable in JSWrapperMap is too costly
​https://bugs.webkit.org/show_bug.cgi?id=194935

Reviewed by Mark Lam.

When we convert Objective-C blocks to JS objects, we need to set up a corresponding function object correctly.
During this allocation, we call [JSValue defineProperty:descriptor] to connect a "prototype" object and "constructor" object.
The problem is that this API has a particularly costly implementation:

_context globalObject][@"Object"] invokeMethod:@"defineProperty" withArguments:@[ self, key, descriptor ?;

This wraps each JS objects appear in this code with Objective-C wrapper. And we convert a NSDictionary to JSObject, which
has "writable", "enumerable", "configurable", "value" fields, and call the "defineProperty" JS function through Objective-C wrapper.
This allocates many Objective-C wrappers and JS objects for descriptors. Since JSC has a direct C++ API "defineOwnProperty", we should
bypass these Objective-C APIs and call JSC's code directly.

This patch changes putNonEnumerable implementation, from calling [JSValue defineProperty:descriptor] to calling JSC C++ code directly.
We do not change [JSValue defineProperty:descriptor] implementation for now because of two reasons. (1) This is not used in our benchmarks
except for this (converting an Objective-C block to a JS object) one path. And (2) even if we were to re-write [JSValue defineProperty:descriptor]
to be more optimized, we would still want to call the JSC C++ version of defineProperty directly here to avoid NSDictionary allocation for a descriptor.

• API/APIUtils.h:

(setException):

• API/JSWrapperMap.mm:

(putNonEnumerable):
(copyMethodsToObject):
(-[JSObjCClassInfo allocateConstructorAndPrototypeInContext:]):
(-[JSObjCClassInfo wrapperForObject:inContext:]):

Location:

trunk/Source/JavaScriptCore

Files:

• API/APIUtils.h (modified) (1 diff)
• API/JSWrapperMap.mm (modified) (5 diffs)
• ChangeLog (modified) (1 diff)

trunk/Source/JavaScriptCore/API/APIUtils.h

@@ -58 +58 @@
         *returnedExceptionRef = toRef(exec, exception);
 #if ENABLE(REMOTE_INSPECTOR)
-    VM& vm = exec->vm();
+    JSC::VM& vm = exec->vm();
     vm.vmEntryGlobalObject(exec)->inspectorController().reportAPIException(exec, JSC::Exception::create(vm, exception));
 #endif

trunk/Source/JavaScriptCore/API/JSWrapperMap.mm

@@ -29 +29 @@
 #if JSC_OBJC_API_ENABLED
 #import "APICast.h"
+#import "APIUtils.h"
 #import "JSAPIWrapperObject.h"
 #import "JSCInlines.h"
@@ -179 +180 @@
 }
 
-inline void putNonEnumerable(JSValue *base, NSString *propertyName, JSValue *value)
-{
-    [base defineProperty:propertyName descriptor:@{
-        JSPropertyDescriptorValueKey: value,
-        JSPropertyDescriptorWritableKey: @YES,
-        JSPropertyDescriptorEnumerableKey: @NO,
-        JSPropertyDescriptorConfigurableKey: @YES
-    }];
+inline void putNonEnumerable(JSContext *context, JSValue *base, NSString *propertyName, JSValue *value)
+{
+    if (![base isObject])
+        return;
+    JSC::ExecState* exec = toJS([context JSGlobalContextRef]);
+    JSC::VM& vm = exec->vm();
+    JSC::JSLockHolder locker(vm);
+    auto scope = DECLARE_CATCH_SCOPE(vm);
+
+    JSC::JSObject* baseObject = JSC::asObject(toJS(exec, [base JSValueRef]));
+    auto name = OpaqueJSString::tryCreate(propertyName);
+    if (!name)
+        return;
+
+    JSC::PropertyDescriptor descriptor;
+    descriptor.setValue(toJS(exec, [value JSValueRef]));
+    descriptor.setEnumerable(false);
+    descriptor.setConfigurable(true);
+    descriptor.setWritable(true);
+    bool shouldThrow = false;
+    baseObject->methodTable(vm)->defineOwnProperty(baseObject, exec, name->identifier(&vm), descriptor, shouldThrow);
+
+    JSValueRef exception = 0;
+    if (handleExceptionIfNeeded(scope, exec, &exception) == ExceptionStatus::DidThrow)
+        [context valueFromNotifyException:exception];
 }
 
@@ -255 +273 @@
             JSObjectRef method = objCCallbackFunctionForMethod(context, objcClass, protocol, isInstanceMethod, sel, types);
             if (method)
-                putNonEnumerable(object, name, [JSValue valueWithJSValueRef:method inContext:context]);
+                putNonEnumerable(context, object, name, [JSValue valueWithJSValueRef:method inContext:context]);
         }
     });
@@ -485 +503 @@
         JSValue* prototype = [JSValue valueWithJSValueRef:toRef(jsPrototype) inContext:context];
         JSValue* constructor = [JSValue valueWithJSValueRef:toRef(jsConstructor) inContext:context];
-        putNonEnumerable(prototype, @"constructor", constructor);
-        putNonEnumerable(constructor, @"prototype", prototype);
+
+        putNonEnumerable(context, prototype, @"constructor", constructor);
+        putNonEnumerable(context, constructor, @"prototype", prototype);
 
         Protocol *exportProtocol = getJSExportProtocol();
@@ -512 +531 @@
             JSValue *constructor = [JSValue valueWithJSValueRef:method inContext:context];
             JSValue *prototype = [JSValue valueWithNewObjectInContext:context];
-            putNonEnumerable(constructor, @"prototype", prototype);
-            putNonEnumerable(prototype, @"constructor", constructor);
+            putNonEnumerable(context, constructor, @"prototype", prototype);
+            putNonEnumerable(context, prototype, @"constructor", constructor);
             return toJS(method);
         }

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-02-22  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] putNonEnumerable in JSWrapperMap is too costly
+        https://bugs.webkit.org/show_bug.cgi?id=194935
+
+        Reviewed by Mark Lam.
+
+        When we convert Objective-C blocks to JS objects, we need to set up a corresponding function object correctly.
+        During this allocation, we call [JSValue defineProperty:descriptor] to connect a "prototype" object and "constructor" object.
+        The problem is that this API has a particularly costly implementation:
+
+            [[_context globalObject][@"Object"] invokeMethod:@"defineProperty" withArguments:@[ self, key, descriptor ]];
+
+        This wraps each JS objects appear in this code with Objective-C wrapper. And we convert a NSDictionary to JSObject, which
+        has "writable", "enumerable", "configurable", "value" fields, and call the "defineProperty" JS function through Objective-C wrapper.
+        This allocates many Objective-C wrappers and JS objects for descriptors. Since JSC has a direct C++ API "defineOwnProperty", we should
+        bypass these Objective-C APIs and call JSC's code directly.
+
+        This patch changes `putNonEnumerable` implementation, from calling [JSValue defineProperty:descriptor] to calling JSC C++ code directly.
+        We do not change [JSValue defineProperty:descriptor] implementation for now because of two reasons. (1) This is not used in our benchmarks
+        except for this (converting an Objective-C block to a JS object) one path. And (2) even if we were to re-write [JSValue defineProperty:descriptor]
+        to be more optimized, we would still want to call the JSC C++ version of defineProperty directly here to avoid NSDictionary allocation for a descriptor.
+
+        * API/APIUtils.h:
+        (setException):
+        * API/JSWrapperMap.mm:
+        (putNonEnumerable):
+        (copyMethodsToObject):
+        (-[JSObjCClassInfo allocateConstructorAndPrototypeInContext:]):
+        (-[JSObjCClassInfo wrapperForObject:inContext:]):
+
 2019-02-22  Yusuke Suzuki  <ysuzuki@apple.com>