Diff
Modified: trunk/JSTests/ChangeLog (240039 => 240040)
--- trunk/JSTests/ChangeLog 2019-01-16 17:11:47 UTC (rev 240039)
+++ trunk/JSTests/ChangeLog 2019-01-16 18:10:44 UTC (rev 240040)
@@ -1,3 +1,16 @@
+2019-01-15 Mark Lam <mark....@apple.com>
+
+ 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-15 Yusuke Suzuki <yusukesuz...@slowstart.org>
[JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)
Added: trunk/JSTests/microbenchmarks/sinkable-new-object-with-builtin-constructor.js (0 => 240040)
--- trunk/JSTests/microbenchmarks/sinkable-new-object-with-builtin-constructor.js (rev 0)
+++ trunk/JSTests/microbenchmarks/sinkable-new-object-with-builtin-constructor.js 2019-01-16 18:10:44 UTC (rev 240040)
@@ -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: trunk/JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js (0 => 240040)
--- trunk/JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js (rev 0)
+++ trunk/JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js 2019-01-16 18:10:44 UTC (rev 240040)
@@ -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: trunk/JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js (0 => 240040)
--- trunk/JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js (rev 0)
+++ trunk/JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js 2019-01-16 18:10:44 UTC (rev 240040)
@@ -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: trunk/JSTests/stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js (0 => 240040)
--- trunk/JSTests/stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js (rev 0)
+++ trunk/JSTests/stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js 2019-01-16 18:10:44 UTC (rev 240040)
@@ -0,0 +1,9 @@
+Object.defineProperty(Function.prototype, 'prototype', {
+ get: function () {
+ throw new Error('hello');
+ }
+});
+
+new Promise(resolve => {
+ new resolve();
+});
Modified: trunk/Source/_javascript_Core/ChangeLog (240039 => 240040)
--- trunk/Source/_javascript_Core/ChangeLog 2019-01-16 17:11:47 UTC (rev 240039)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-01-16 18:10:44 UTC (rev 240040)
@@ -1,3 +1,26 @@
+2019-01-15 Mark Lam <mark....@apple.com>
+
+ 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-15 Yusuke Suzuki <yusukesuz...@slowstart.org>
[JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (240039 => 240040)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2019-01-16 17:11:47 UTC (rev 240039)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2019-01-16 18:10:44 UTC (rev 240040)
@@ -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: trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp (240039 => 240040)
--- trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2019-01-16 17:11:47 UTC (rev 240039)
+++ trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2019-01-16 18:10:44 UTC (rev 240040)
@@ -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: trunk/Source/_javascript_Core/runtime/JSFunctionInlines.h (240039 => 240040)
--- trunk/Source/_javascript_Core/runtime/JSFunctionInlines.h 2019-01-16 17:11:47 UTC (rev 240039)
+++ trunk/Source/_javascript_Core/runtime/JSFunctionInlines.h 2019-01-16 18:10:44 UTC (rev 240040)
@@ -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: trunk/Source/_javascript_Core/runtime/PropertySlot.h (240039 => 240040)
--- trunk/Source/_javascript_Core/runtime/PropertySlot.h 2019-01-16 17:11:47 UTC (rev 240039)
+++ trunk/Source/_javascript_Core/runtime/PropertySlot.h 2019-01-16 18:10:44 UTC (rev 240040)
@@ -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
