Attachment 341216 Details for Bug 185954 – the patch

[patch] the patch

blah.patch (text/plain), 7.03 KB, created by Filip Pizlo on 2018-05-24 12:35:24 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2018-05-24 12:35:24 PDT

Size: 7.03 KB

patch

obsolete

>Index: Source/JavaScriptCore/runtime/JSString.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/JSString.cpp	(revision 232133)
>+++ Source/JavaScriptCore/runtime/JSString.cpp	(working copy)
>@@ -1,7 +1,7 @@
> /*
>  *  Copyright (C) 1999-2002 Harri Porten (porten@kde.org)
>  *  Copyright (C) 2001 Peter Kelly (pmk@post.com)
>- *  Copyright (C) 2004-2017 Apple Inc. All rights reserved.
>+ *  Copyright (C) 2004-2018 Apple Inc. All rights reserved.
>  *
>  *  This library is free software; you can redistribute it and/or
>  *  modify it under the terms of the GNU Library General Public
>@@ -286,35 +286,92 @@ void JSRopeString::resolveRope(ExecState
>     ASSERT(!isRope());
> }
> 
>-// Overview: These functions convert a JSString from holding a string in rope form
>-// down to a simple String representation. It does so by building up the string
>-// backwards, since we want to avoid recursion, we expect that the tree structure
>-// representing the rope is likely imbalanced with more nodes down the left side
>-// (since appending to the string is likely more common) - and as such resolving
>-// in this fashion should minimize work queue size.  (If we built the queue forwards
>-// we would likely have to place all of the constituent StringImpls into the
>-// Vector before performing any concatenation, but by working backwards we likely
>-// only fill the queue with the number of substrings at any given level in a
>-// rope-of-ropes.)
>-void JSRopeString::resolveRopeSlowCase8(LChar* buffer) const
>-{
>-    LChar* position = buffer + length(); // We will be working backwards over the rope.
>-    Vector<JSString*, 32, UnsafeVectorOverflow> workQueue; // Putting strings into a Vector is only OK because there are no GC points in this method.
>+class JSRopeString::FastResolvingWorklist {
>+public:
>+    static constexpr unsigned capacity = 32;
>     
>-    for (size_t i = 0; i < s_maxInternalRopeLength && fiber(i); ++i)
>-        workQueue.append(fiber(i).get());
>+    ALWAYS_INLINE FastResolvingWorklist()
>+    {
>+    }
>+    
>+    ALWAYS_INLINE void pushUnchecked(JSString* string)
>+    {
>+        ASSERT(m_size < capacity);
>+        m_strings[m_size++] = string;
>+    }
>+    
>+    ALWAYS_INLINE bool push(JSString* string)
>+    {
>+        if (m_size == capacity)
>+            return false;
>+        m_strings[m_size++] = string;
>+        return true;
>+    }
>+    
>+    ALWAYS_INLINE JSString* pop()
>+    {
>+        if (!m_size)
>+            return nullptr;
>+        return m_strings[--m_size];
>+    }
>+    
>+    ALWAYS_INLINE unsigned size() const { return m_size; }
>+    
>+    ALWAYS_INLINE JSString* operator[](unsigned i) const { return m_strings[i]; }
>+    
>+private:
>+    JSString* m_strings[capacity];
>+    unsigned m_size { 0 };
>+};
> 
>-    while (!workQueue.isEmpty()) {
>-        JSString* currentFiber = workQueue.last();
>-        workQueue.removeLast();
>+class JSRopeString::SlowResolvingWorklist {
>+public:
>+    ALWAYS_INLINE SlowResolvingWorklist(const FastResolvingWorklist& otherWorklist, JSString* anotherString)
>+    {
>+        for (unsigned i = 0; i < otherWorklist.size(); ++i)
>+            m_vector.append(otherWorklist[i]);
>+        m_vector.append(anotherString);
>+    }
>+    
>+    SlowResolvingWorklist(const SlowResolvingWorklist&, JSString*)
>+    {
>+        UNREACHABLE_FOR_PLATFORM();
>+    }
>+    
>+    ALWAYS_INLINE bool push(JSString* string)
>+    {
>+        m_vector.append(string);
>+        return true;
>+    }
>+    
>+    ALWAYS_INLINE JSString* pop()
>+    {
>+        if (m_vector.isEmpty())
>+            return nullptr;
>+        return m_vector.takeLast();
>+    }
>+
>+private:
>+    Vector<JSString*, FastResolvingWorklist::capacity * 2, UnsafeVectorOverflow> m_vector; // Putting strings into a Vector is only OK because there are no GC points in this method.
>+};
> 
>+template<typename WorklistType>
>+ALWAYS_INLINE void JSRopeString::resolveRopeSlowCase8Generic(LChar*& position, WorklistType& worklist) const
>+{
>+    while (JSString* currentFiber = worklist.pop()) {
>         const LChar* characters;
>         
>         if (currentFiber->isRope()) {
>             JSRopeString* currentFiberAsRope = static_cast<JSRopeString*>(currentFiber);
>             if (!currentFiberAsRope->isSubstring()) {
>-                for (size_t i = 0; i < s_maxInternalRopeLength && currentFiberAsRope->fiber(i); ++i)
>-                    workQueue.append(currentFiberAsRope->fiber(i).get());
>+                for (size_t i = 0; i < s_maxInternalRopeLength && currentFiberAsRope->fiber(i); ++i) {
>+                    JSString* anotherString = currentFiberAsRope->fiber(i).get();
>+                    if (UNLIKELY(!worklist.push(anotherString))) {
>+                        SlowResolvingWorklist slowWorklist(worklist, anotherString);
>+                        resolveRopeSlowCase8Generic(position, slowWorklist);
>+                        return;
>+                    }
>+                }
>                 continue;
>             }
>             ASSERT(!currentFiberAsRope->substringBase()->isRope());
>@@ -328,7 +385,28 @@ void JSRopeString::resolveRopeSlowCase8(
>         position -= length;
>         StringImpl::copyCharacters(position, characters, length);
>     }
>+}
>+
>+// Overview: These functions convert a JSString from holding a string in rope form
>+// down to a simple String representation. It does so by building up the string
>+// backwards, since we want to avoid recursion, we expect that the tree structure
>+// representing the rope is likely imbalanced with more nodes down the left side
>+// (since appending to the string is likely more common) - and as such resolving
>+// in this fashion should minimize work queue size.  (If we built the queue forwards
>+// we would likely have to place all of the constituent StringImpls into the
>+// Vector before performing any concatenation, but by working backwards we likely
>+// only fill the queue with the number of substrings at any given level in a
>+// rope-of-ropes.)
>+void JSRopeString::resolveRopeSlowCase8(LChar* buffer) const
>+{
>+    LChar* position = buffer + length(); // We will be working backwards over the rope.
>+    FastResolvingWorklist worklist;
> 
>+    for (size_t i = 0; i < s_maxInternalRopeLength && fiber(i); ++i)
>+        worklist.pushUnchecked(fiber(i).get());
>+
>+    resolveRopeSlowCase8Generic(position, worklist);
>+    
>     ASSERT(buffer == position);
> }
> 
>Index: Source/JavaScriptCore/runtime/JSString.h
>===================================================================
>--- Source/JavaScriptCore/runtime/JSString.h	(revision 232133)
>+++ Source/JavaScriptCore/runtime/JSString.h	(working copy)
>@@ -421,6 +421,10 @@ private:
>     JS_EXPORT_PRIVATE void resolveRope(ExecState*) const;
>     JS_EXPORT_PRIVATE void resolveRopeToAtomicString(ExecState*) const;
>     JS_EXPORT_PRIVATE RefPtr<AtomicStringImpl> resolveRopeToExistingAtomicString(ExecState*) const;
>+    template<typename WorklistType>
>+    void resolveRopeSlowCase8Generic(LChar*&, WorklistType&) const;
>+    class FastResolvingWorklist;
>+    class SlowResolvingWorklist;
>     void resolveRopeSlowCase8(LChar*) const;
>     void resolveRopeSlowCase(UChar*) const;
>     void outOfMemory(ExecState*) const;

Actions: View | Formatted Diff | Diff

Attachments on bug 185954: 341216