Attachment 343288 Details for Bug 186898 – patch

[patch] patch

c-backup.diff (text/plain), 8.22 KB, created by Saam Barati on 2018-06-21 16:34:18 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-06-21 16:34:18 PDT

Size: 8.22 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 233066)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,33 @@
>+2018-06-21  Saam Barati  <sbarati@apple.com>
>+
>+        ensureWritableX should only convert away from CoW when it will succeed
>+        https://bugs.webkit.org/show_bug.cgi?id=186898
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Otherwise, when we OSR exit, we'll end up profiling the array after
>+        it has been converted away from CoW. It's better for the ArrayProfile
>+        to see the array as it's still in CoW mode.
>+        
>+        This patch also renames ensureWritableX to tryMakeWritableX since these
>+        were never really "ensure" operations -- they may fail and return null.
>+
>+        * dfg/DFGOperations.cpp:
>+        * runtime/JSObject.cpp:
>+        (JSC::JSObject::tryMakeWritableInt32Slow):
>+        (JSC::JSObject::tryMakeWritableDoubleSlow):
>+        (JSC::JSObject::tryMakeWritableContiguousSlow):
>+        (JSC::JSObject::ensureWritableInt32Slow): Deleted.
>+        (JSC::JSObject::ensureWritableDoubleSlow): Deleted.
>+        (JSC::JSObject::ensureWritableContiguousSlow): Deleted.
>+        * runtime/JSObject.h:
>+        (JSC::JSObject::tryMakeWritableInt32):
>+        (JSC::JSObject::tryMakeWritableDouble):
>+        (JSC::JSObject::tryMakeWritableContiguous):
>+        (JSC::JSObject::ensureWritableInt32): Deleted.
>+        (JSC::JSObject::ensureWritableDouble): Deleted.
>+        (JSC::JSObject::ensureWritableContiguous): Deleted.
>+
> 2018-06-21  Saam Barati  <sbarati@apple.com>
> 
>         Do some CoW cleanup
>Index: Source/JavaScriptCore/dfg/DFGOperations.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOperations.cpp	(revision 233065)
>+++ Source/JavaScriptCore/dfg/DFGOperations.cpp	(working copy)
>@@ -1888,7 +1888,7 @@ char* JIT_OPERATION operationEnsureInt32
>     if (!cell->isObject())
>         return 0;
> 
>-    auto* result = reinterpret_cast<char*>(asObject(cell)->ensureWritableInt32(vm).data());
>+    auto* result = reinterpret_cast<char*>(asObject(cell)->tryMakeWritableInt32(vm).data());
>     ASSERT((!isCopyOnWrite(asObject(cell)->indexingMode()) && hasInt32(cell->indexingMode())) || !result);
>     return result;
> }
>@@ -1901,7 +1901,7 @@ char* JIT_OPERATION operationEnsureDoubl
>     if (!cell->isObject())
>         return 0;
> 
>-    auto* result = reinterpret_cast<char*>(asObject(cell)->ensureWritableDouble(vm).data());
>+    auto* result = reinterpret_cast<char*>(asObject(cell)->tryMakeWritableDouble(vm).data());
>     ASSERT((!isCopyOnWrite(asObject(cell)->indexingMode()) && hasDouble(cell->indexingMode())) || !result);
>     return result;
> }
>@@ -1914,7 +1914,7 @@ char* JIT_OPERATION operationEnsureConti
>     if (!cell->isObject())
>         return 0;
>     
>-    auto* result = reinterpret_cast<char*>(asObject(cell)->ensureWritableContiguous(vm).data());
>+    auto* result = reinterpret_cast<char*>(asObject(cell)->tryMakeWritableContiguous(vm).data());
>     ASSERT((!isCopyOnWrite(asObject(cell)->indexingMode()) && hasContiguous(cell->indexingMode())) || !result);
>     return result;
> }
>Index: Source/JavaScriptCore/runtime/JSObject.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/JSObject.cpp	(revision 233065)
>+++ Source/JavaScriptCore/runtime/JSObject.cpp	(working copy)
>@@ -1474,14 +1474,16 @@ void JSObject::convertDoubleToContiguous
>     setIndexQuickly(vm, index, value);
> }
> 
>-ContiguousJSValues JSObject::ensureWritableInt32Slow(VM& vm)
>+ContiguousJSValues JSObject::tryMakeWritableInt32Slow(VM& vm)
> {
>     ASSERT(inherits(vm, info()));
> 
>     if (isCopyOnWrite(indexingMode())) {
>-        convertFromCopyOnWrite(vm);
>-        if (hasInt32(indexingMode()))
>+        if (hasInt32(indexingMode())) {
>+            convertFromCopyOnWrite(vm);
>             return butterfly()->contiguousInt32();
>+        }
>+        return ContiguousJSValues();
>     }
> 
>     if (structure(vm)->hijacksIndexingHeader())
>@@ -1507,14 +1509,18 @@ ContiguousJSValues JSObject::ensureWrita
>     }
> }
> 
>-ContiguousDoubles JSObject::ensureWritableDoubleSlow(VM& vm)
>+ContiguousDoubles JSObject::tryMakeWritableDoubleSlow(VM& vm)
> {
>     ASSERT(inherits(vm, info()));
> 
>     if (isCopyOnWrite(indexingMode())) {
>-        convertFromCopyOnWrite(vm);
>-        if (hasDouble(indexingMode()))
>-            return butterfly()->contiguousDouble();
>+        if (hasDouble(indexingMode()) || hasInt32(indexingMode())) {
>+            convertFromCopyOnWrite(vm);
>+            if (hasDouble(indexingMode()))
>+                return butterfly()->contiguousDouble();
>+            ASSERT(hasInt32(indexingMode()));
>+        } else
>+            return ContiguousDoubles();
>     }
> 
>     if (structure(vm)->hijacksIndexingHeader())
>@@ -1542,14 +1548,18 @@ ContiguousDoubles JSObject::ensureWritab
>     }
> }
> 
>-ContiguousJSValues JSObject::ensureWritableContiguousSlow(VM& vm)
>+ContiguousJSValues JSObject::tryMakeWritableContiguousSlow(VM& vm)
> {
>     ASSERT(inherits(vm, info()));
> 
>     if (isCopyOnWrite(indexingMode())) {
>-        convertFromCopyOnWrite(vm);
>-        if (hasContiguous(indexingMode()))
>-            return butterfly()->contiguous();
>+        if (hasContiguous(indexingMode()) || hasInt32(indexingMode()) || hasDouble(indexingMode())) {
>+            convertFromCopyOnWrite(vm);
>+            if (hasContiguous(indexingMode()))
>+                return butterfly()->contiguous();
>+            ASSERT(hasInt32(indexingMode()) || hasDouble(indexingMode()));
>+        } else
>+            return ContiguousJSValues();
>     }
> 
>     if (structure(vm)->hijacksIndexingHeader())
>Index: Source/JavaScriptCore/runtime/JSObject.h
>===================================================================
>--- Source/JavaScriptCore/runtime/JSObject.h	(revision 233065)
>+++ Source/JavaScriptCore/runtime/JSObject.h	(working copy)
>@@ -822,34 +822,34 @@ public:
>     // indexing should be sparse, we're having a bad time, or because
>     // we already have a more general form of storage (double,
>     // contiguous, array storage).
>-    ContiguousJSValues ensureWritableInt32(VM& vm)
>+    ContiguousJSValues tryMakeWritableInt32(VM& vm)
>     {
>         if (LIKELY(hasInt32(indexingType()) && !isCopyOnWrite(indexingMode())))
>             return m_butterfly->contiguousInt32();
>             
>-        return ensureWritableInt32Slow(vm);
>+        return tryMakeWritableInt32Slow(vm);
>     }
>         
>     // Returns 0 if double storage cannot be created - either because
>     // indexing should be sparse, we're having a bad time, or because
>     // we already have a more general form of storage (contiguous,
>     // or array storage).
>-    ContiguousDoubles ensureWritableDouble(VM& vm)
>+    ContiguousDoubles tryMakeWritableDouble(VM& vm)
>     {
>         if (LIKELY(hasDouble(indexingType()) && !isCopyOnWrite(indexingMode())))
>             return m_butterfly->contiguousDouble();
>             
>-        return ensureWritableDoubleSlow(vm);
>+        return tryMakeWritableDoubleSlow(vm);
>     }
>         
>     // Returns 0 if contiguous storage cannot be created - either because
>     // indexing should be sparse or because we're having a bad time.
>-    ContiguousJSValues ensureWritableContiguous(VM& vm)
>+    ContiguousJSValues tryMakeWritableContiguous(VM& vm)
>     {
>         if (LIKELY(hasContiguous(indexingType()) && !isCopyOnWrite(indexingMode())))
>             return m_butterfly->contiguous();
>             
>-        return ensureWritableContiguousSlow(vm);
>+        return tryMakeWritableContiguousSlow(vm);
>     }
> 
>     // Ensure that the object is in a mode where it has array storage. Use
>@@ -1059,9 +1059,9 @@ private:
>         
>     bool ensureLengthSlow(VM&, unsigned length);
>         
>-    ContiguousJSValues ensureWritableInt32Slow(VM&);
>-    ContiguousDoubles ensureWritableDoubleSlow(VM&);
>-    ContiguousJSValues ensureWritableContiguousSlow(VM&);
>+    ContiguousJSValues tryMakeWritableInt32Slow(VM&);
>+    ContiguousDoubles tryMakeWritableDoubleSlow(VM&);
>+    ContiguousJSValues tryMakeWritableContiguousSlow(VM&);
>     JS_EXPORT_PRIVATE ArrayStorage* ensureArrayStorageSlow(VM&);
> 
>     PropertyOffset prepareToPutDirectWithoutTransition(VM&, PropertyName, unsigned attributes, StructureID, Structure*);

Flags:

keith_miller: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 186898: 343288 | 343290