Title: [240378] branches/safari-607-branch
Revision
240378
Author
[email protected]
Date
2019-01-23 17:21:26 -0800 (Wed, 23 Jan 2019)

Log Message

Cherry-pick r240040. rdar://problem/47458365

    JSFunction::canUseAllocationProfile() should account for builtin functions with no own prototypes.
    https://bugs.webkit.org/show_bug.cgi?id=193423
    <rdar://problem/46209355>

    Reviewed by Saam Barati.

    JSTests:

    * microbenchmarks/sinkable-new-object-with-builtin-constructor.js: Added.
    * stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js: Added.
    * stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js: Added.
    * stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js: Added.

    Source/_javascript_Core:

    JSFunction::canUseAllocationProfile() should return false for most builtins
    because the majority of them have no prototype property.  The only exception to
    this is the few builtin functions that are explicitly used as constructors.

    For these builtin constructors, JSFunction::canUseAllocationProfile() should also
    return false if the prototype property is a getter or custom getter because
    getting the prototype would then be effectful.

    * dfg/DFGOperations.cpp:
    * runtime/CommonSlowPaths.cpp:
    (JSC::SLOW_PATH_DECL):
    * runtime/JSFunctionInlines.h:
    (JSC::JSFunction::canUseAllocationProfile):
    * runtime/PropertySlot.h:

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240040 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Added Paths

Diff

Modified: branches/safari-607-branch/JSTests/ChangeLog (240377 => 240378)


--- branches/safari-607-branch/JSTests/ChangeLog	2019-01-24 01:21:22 UTC (rev 240377)
+++ branches/safari-607-branch/JSTests/ChangeLog	2019-01-24 01:21:26 UTC (rev 240378)
@@ -1,5 +1,56 @@
 2019-01-23  Alan Coon  <[email protected]>
 
+        Cherry-pick r240040. rdar://problem/47458365
+
+    JSFunction::canUseAllocationProfile() should account for builtin functions with no own prototypes.
+    https://bugs.webkit.org/show_bug.cgi?id=193423
+    <rdar://problem/46209355>
+    
+    Reviewed by Saam Barati.
+    
+    JSTests:
+    
+    * microbenchmarks/sinkable-new-object-with-builtin-constructor.js: Added.
+    * stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js: Added.
+    * stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js: Added.
+    * stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js: Added.
+    
+    Source/_javascript_Core:
+    
+    JSFunction::canUseAllocationProfile() should return false for most builtins
+    because the majority of them have no prototype property.  The only exception to
+    this is the few builtin functions that are explicitly used as constructors.
+    
+    For these builtin constructors, JSFunction::canUseAllocationProfile() should also
+    return false if the prototype property is a getter or custom getter because
+    getting the prototype would then be effectful.
+    
+    * dfg/DFGOperations.cpp:
+    * runtime/CommonSlowPaths.cpp:
+    (JSC::SLOW_PATH_DECL):
+    * runtime/JSFunctionInlines.h:
+    (JSC::JSFunction::canUseAllocationProfile):
+    * runtime/PropertySlot.h:
+    
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240040 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-01-15  Mark Lam  <[email protected]>
+
+            JSFunction::canUseAllocationProfile() should account for builtin functions with no own prototypes.
+            https://bugs.webkit.org/show_bug.cgi?id=193423
+            <rdar://problem/46209355>
+
+            Reviewed by Saam Barati.
+
+            * microbenchmarks/sinkable-new-object-with-builtin-constructor.js: Added.
+            * stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js: Added.
+            * stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js: Added.
+            * stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js: Added.
+
+2019-01-23  Alan Coon  <[email protected]>
+
         Cherry-pick r240024. rdar://problem/47458299
 
     [JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)

Added: branches/safari-607-branch/JSTests/microbenchmarks/sinkable-new-object-with-builtin-constructor.js (0 => 240378)


--- branches/safari-607-branch/JSTests/microbenchmarks/sinkable-new-object-with-builtin-constructor.js	                        (rev 0)
+++ branches/safari-607-branch/JSTests/microbenchmarks/sinkable-new-object-with-builtin-constructor.js	2019-01-24 01:21:26 UTC (rev 240378)
@@ -0,0 +1,28 @@
+function sumOfArithSeries(limit) {
+    return limit * (limit + 1) / 2;
+}
+
+var n = 10000000;
+
+function bar() { }
+
+function foo(b) {
+    var result = 0;
+    for (var i = 0; i < n; ++i) {
+        var iter = ([i, i + 1])[Symbol.iterator]();
+        if (b) {
+            bar(o, p);
+            return;
+        }
+        for (x of iter)
+            result += x;
+    }
+    return result;
+}
+
+noInline(bar);
+noInline(foo);
+
+var result = foo(false);
+if (result != sumOfArithSeries(n - 1) + sumOfArithSeries(n))
+    throw "Error: bad result: " + result;

Added: branches/safari-607-branch/JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js (0 => 240378)


--- branches/safari-607-branch/JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js	                        (rev 0)
+++ branches/safari-607-branch/JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js	2019-01-24 01:21:26 UTC (rev 240378)
@@ -0,0 +1,15 @@
+var invokeCount = 0;
+
+Object.defineProperty(Function.prototype, 'prototype', {
+    get: function () {
+        invokeCount++;
+    }
+});
+
+new Promise(resolve => {
+    for (var i = 0; i < 10000; ++i)
+        new resolve();
+
+    if (invokeCount != 10000)
+        $vm.crash();
+});

Added: branches/safari-607-branch/JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js (0 => 240378)


--- branches/safari-607-branch/JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js	                        (rev 0)
+++ branches/safari-607-branch/JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js	2019-01-24 01:21:26 UTC (rev 240378)
@@ -0,0 +1,15 @@
+new Promise(resolve => {
+    var invokeCount = 0;
+
+    Object.defineProperty(resolve, 'prototype', {
+        get: function () {
+            invokeCount++;
+        }
+    });
+
+    for (var i = 0; i < 10000; ++i)
+        new resolve();
+
+    if (invokeCount != 10000)
+        $vm.crash();
+});

Added: branches/safari-607-branch/JSTests/stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js (0 => 240378)


--- branches/safari-607-branch/JSTests/stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js	                        (rev 0)
+++ branches/safari-607-branch/JSTests/stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js	2019-01-24 01:21:26 UTC (rev 240378)
@@ -0,0 +1,9 @@
+Object.defineProperty(Function.prototype, 'prototype', {
+    get: function () {
+        throw new Error('hello');
+    }
+});
+
+new Promise(resolve => {
+    new resolve();
+});

Modified: branches/safari-607-branch/Source/_javascript_Core/ChangeLog (240377 => 240378)


--- branches/safari-607-branch/Source/_javascript_Core/ChangeLog	2019-01-24 01:21:22 UTC (rev 240377)
+++ branches/safari-607-branch/Source/_javascript_Core/ChangeLog	2019-01-24 01:21:26 UTC (rev 240378)
@@ -1,5 +1,66 @@
 2019-01-23  Alan Coon  <[email protected]>
 
+        Cherry-pick r240040. rdar://problem/47458365
+
+    JSFunction::canUseAllocationProfile() should account for builtin functions with no own prototypes.
+    https://bugs.webkit.org/show_bug.cgi?id=193423
+    <rdar://problem/46209355>
+    
+    Reviewed by Saam Barati.
+    
+    JSTests:
+    
+    * microbenchmarks/sinkable-new-object-with-builtin-constructor.js: Added.
+    * stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js: Added.
+    * stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js: Added.
+    * stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js: Added.
+    
+    Source/_javascript_Core:
+    
+    JSFunction::canUseAllocationProfile() should return false for most builtins
+    because the majority of them have no prototype property.  The only exception to
+    this is the few builtin functions that are explicitly used as constructors.
+    
+    For these builtin constructors, JSFunction::canUseAllocationProfile() should also
+    return false if the prototype property is a getter or custom getter because
+    getting the prototype would then be effectful.
+    
+    * dfg/DFGOperations.cpp:
+    * runtime/CommonSlowPaths.cpp:
+    (JSC::SLOW_PATH_DECL):
+    * runtime/JSFunctionInlines.h:
+    (JSC::JSFunction::canUseAllocationProfile):
+    * runtime/PropertySlot.h:
+    
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240040 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-01-15  Mark Lam  <[email protected]>
+
+            JSFunction::canUseAllocationProfile() should account for builtin functions with no own prototypes.
+            https://bugs.webkit.org/show_bug.cgi?id=193423
+            <rdar://problem/46209355>
+
+            Reviewed by Saam Barati.
+
+            JSFunction::canUseAllocationProfile() should return false for most builtins
+            because the majority of them have no prototype property.  The only exception to
+            this is the few builtin functions that are explicitly used as constructors.
+
+            For these builtin constructors, JSFunction::canUseAllocationProfile() should also
+            return false if the prototype property is a getter or custom getter because
+            getting the prototype would then be effectful.
+
+            * dfg/DFGOperations.cpp:
+            * runtime/CommonSlowPaths.cpp:
+            (JSC::SLOW_PATH_DECL):
+            * runtime/JSFunctionInlines.h:
+            (JSC::JSFunction::canUseAllocationProfile):
+            * runtime/PropertySlot.h:
+
+2019-01-23  Alan Coon  <[email protected]>
+
         Cherry-pick r240024. rdar://problem/47458299
 
     [JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)

Modified: branches/safari-607-branch/Source/_javascript_Core/dfg/DFGOperations.cpp (240377 => 240378)


--- branches/safari-607-branch/Source/_javascript_Core/dfg/DFGOperations.cpp	2019-01-24 01:21:22 UTC (rev 240377)
+++ branches/safari-607-branch/Source/_javascript_Core/dfg/DFGOperations.cpp	2019-01-24 01:21:26 UTC (rev 240378)
@@ -301,7 +301,7 @@
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (constructor->type() == JSFunctionType && jsCast<JSFunction*>(constructor)->canUseAllocationProfile()) {
         auto rareData = jsCast<JSFunction*>(constructor)->ensureRareDataAndAllocationProfile(exec, inlineCapacity);
-        RETURN_IF_EXCEPTION(scope, nullptr);
+        scope.releaseAssertNoException();
         ObjectAllocationProfile* allocationProfile = rareData->objectAllocationProfile();
         Structure* structure = allocationProfile->structure();
         JSObject* result = constructEmptyObject(exec, structure);

Modified: branches/safari-607-branch/Source/_javascript_Core/runtime/CommonSlowPaths.cpp (240377 => 240378)


--- branches/safari-607-branch/Source/_javascript_Core/runtime/CommonSlowPaths.cpp	2019-01-24 01:21:22 UTC (rev 240377)
+++ branches/safari-607-branch/Source/_javascript_Core/runtime/CommonSlowPaths.cpp	2019-01-24 01:21:26 UTC (rev 240378)
@@ -243,6 +243,7 @@
 
         size_t inlineCapacity = bytecode.inlineCapacity;
         ObjectAllocationProfile* allocationProfile = constructor->ensureRareDataAndAllocationProfile(exec, inlineCapacity)->objectAllocationProfile();
+        throwScope.releaseAssertNoException();
         Structure* structure = allocationProfile->structure();
         result = constructEmptyObject(exec, structure);
         if (structure->hasPolyProto()) {

Modified: branches/safari-607-branch/Source/_javascript_Core/runtime/JSFunctionInlines.h (240377 => 240378)


--- branches/safari-607-branch/Source/_javascript_Core/runtime/JSFunctionInlines.h	2019-01-24 01:21:22 UTC (rev 240377)
+++ branches/safari-607-branch/Source/_javascript_Core/runtime/JSFunctionInlines.h	2019-01-24 01:21:26 UTC (rev 240378)
@@ -110,9 +110,17 @@
 
 inline bool JSFunction::canUseAllocationProfile()
 {
-    if (isHostFunction())
-        return false;
+    if (isHostOrBuiltinFunction()) {
+        if (isHostFunction())
+            return false;
 
+        VM& vm = globalObject()->vm();
+        unsigned attributes;
+        JSValue prototype = getDirect(vm, vm.propertyNames->prototype, attributes);
+        if (!prototype || (attributes & PropertyAttribute::AccessorOrCustomAccessorOrValue))
+            return false;
+    }
+
     // If we don't have a prototype property, we're not guaranteed it's
     // non-configurable. For example, user code can define the prototype
     // as a getter. JS semantics require that the getter is called every

Modified: branches/safari-607-branch/Source/_javascript_Core/runtime/PropertySlot.h (240377 => 240378)


--- branches/safari-607-branch/Source/_javascript_Core/runtime/PropertySlot.h	2019-01-24 01:21:22 UTC (rev 240377)
+++ branches/safari-607-branch/Source/_javascript_Core/runtime/PropertySlot.h	2019-01-24 01:21:26 UTC (rev 240378)
@@ -45,6 +45,7 @@
     CustomAccessor    = 1 << 5,
     CustomValue       = 1 << 6,
     CustomAccessorOrValue = CustomAccessor | CustomValue,
+    AccessorOrCustomAccessorOrValue = Accessor | CustomAccessor | CustomValue,
 
     // Things that are used by static hashtables are not in the attributes byte in PropertyMapEntry.
     Function          = 1 << 8,  // property is a function - only used by static hashtables
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to