Attachment 362237 Details for Bug 194762 – ROLLOUT of r241612

[patch] ROLLOUT of r241612

bug-194762-20190217101010.patch (text/plain), 11.79 KB, created by WebKit Commit Bot on 2019-02-17 10:10:10 PST

(hide)

Description:

Filename:

MIME Type:

Creator: WebKit Commit Bot

Created: 2019-02-17 10:10:10 PST

Size: 11.79 KB

patch

obsolete

>Subversion Revision: 241649
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 4a261f11973b098435632261f24a6b6db89bdb14..165c1820f75845e5e77b2981132b8a8ff9decd53 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-02-17  Commit Queue  <commit-queue@webkit.org>
>+
>+        Unreviewed, rolling out r241612.
>+        https://bugs.webkit.org/show_bug.cgi?id=194762
>+
>+        "It regressed JetStream2 parsing tests by ~40%" (Requested by
>+        saamyjoon on #webkit).
>+
>+        Reverted changeset:
>+
>+        "Move bytecode cache-related filesystem code out of CodeCache"
>+        https://bugs.webkit.org/show_bug.cgi?id=194675
>+        https://trac.webkit.org/changeset/241612
>+
> 2019-02-16  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] JSWrapperObject should not be destructible
>diff --git a/Source/JavaScriptCore/jsc.cpp b/Source/JavaScriptCore/jsc.cpp
>index 37ff1d77a5fd29892479956118dd1a3c4d6132ed..2da8f91becab72381586a4076b77b7c83e4c5afb 100644
>--- a/Source/JavaScriptCore/jsc.cpp
>+++ b/Source/JavaScriptCore/jsc.cpp
>@@ -955,110 +955,12 @@ static bool fetchScriptFromLocalFileSystem(const String& fileName, Vector<char>&
>     return true;
> }
> 
>-class ShellSourceProvider : public StringSourceProvider {
>-public:
>-    static Ref<ShellSourceProvider> create(const String& source, const SourceOrigin& sourceOrigin, URL&& url, const TextPosition& startPosition, SourceProviderSourceType sourceType)
>-    {
>-        return adoptRef(*new ShellSourceProvider(source, sourceOrigin, WTFMove(url), startPosition, sourceType));
>-    }
>-
>-    ~ShellSourceProvider()
>-    {
>-#if OS(DARWIN)
>-        if (m_cachedBytecode.size())
>-            munmap(const_cast<void*>(m_cachedBytecode.data()), m_cachedBytecode.size());
>-#endif
>-    }
>-
>-    const CachedBytecode* cachedBytecode() const override
>-    {
>-        return &m_cachedBytecode;
>-    }
>-
>-    bool cacheBytecode(const CachedBytecode& cachedBytecode) const override
>-    {
>-#if OS(DARWIN)
>-        String filename = cachePath();
>-        if (filename.isNull())
>-            return false;
>-        int fd = open(filename.utf8().data(), O_CREAT | O_WRONLY, 0666);
>-        if (fd == -1)
>-            return false;
>-        int rc = flock(fd, LOCK_EX | LOCK_NB);
>-        if (!rc)
>-            write(fd, cachedBytecode.data(), cachedBytecode.size());
>-        close(fd);
>-        return !rc;
>-#else
>-        return false;
>-#endif
>-    }
>-
>-private:
>-    String cachePath() const
>-    {
>-        const char* cachePath = Options::diskCachePath();
>-        if (!cachePath)
>-            return String(static_cast<const char*>(nullptr));
>-
>-        String filename = sourceOrigin().string();
>-        filename.replace('/', '_');
>-        return makeString(cachePath, '/', source().toString().hash(), '-', filename, ".bytecode-cache");
>-    }
>-
>-    void loadBytecode()
>-    {
>-#if OS(DARWIN)
>-        String filename = cachePath();
>-        if (filename.isNull())
>-            return;
>-
>-        int fd = open(filename.utf8().data(), O_RDONLY);
>-        if (fd == -1)
>-            return;
>-
>-        int rc = flock(fd, LOCK_SH | LOCK_NB);
>-        if (rc) {
>-            close(fd);
>-            return;
>-        }
>-
>-        struct stat sb;
>-        int res = fstat(fd, &sb);
>-        size_t size = static_cast<size_t>(sb.st_size);
>-        if (res || !size) {
>-            close(fd);
>-            return;
>-        }
>-
>-        void* buffer = mmap(nullptr, size, PROT_READ, MAP_PRIVATE, fd, 0);
>-        close(fd);
>-        if (buffer == MAP_FAILED)
>-            return;
>-        m_cachedBytecode = CachedBytecode { buffer, size };
>-#endif
>-    }
>-
>-    ShellSourceProvider(const String& source, const SourceOrigin& sourceOrigin, URL&& url, const TextPosition& startPosition, SourceProviderSourceType sourceType)
>-        : StringSourceProvider(source, sourceOrigin, WTFMove(url), startPosition, sourceType)
>-    {
>-        loadBytecode();
>-    }
>-
>-    CachedBytecode m_cachedBytecode;
>-};
>-
>-static inline SourceCode jscSource(const String& source, const SourceOrigin& sourceOrigin, URL&& url = URL(), const TextPosition& startPosition = TextPosition(), SourceProviderSourceType sourceType = SourceProviderSourceType::Program)
>-{
>-    return SourceCode(ShellSourceProvider::create(source, sourceOrigin, WTFMove(url), startPosition, sourceType), startPosition.m_line.oneBasedInt(), startPosition.m_column.oneBasedInt());
>-}
>-
> template<typename Vector>
> static inline SourceCode jscSource(const Vector& utf8, const SourceOrigin& sourceOrigin, const String& filename)
> {
>     // FIXME: This should use an absolute file URL https://bugs.webkit.org/show_bug.cgi?id=193077
>     String str = stringFromUTF(utf8);
>-    return jscSource(str, sourceOrigin, URL({ }, filename));
>+    return makeSource(str, sourceOrigin, URL({ }, filename));
> }
> 
> template<typename Vector>
>@@ -1141,7 +1043,7 @@ JSInternalPromise* GlobalObject::moduleLoaderFetch(JSGlobalObject* globalObject,
>     }
> #endif
> 
>-    auto sourceCode = JSSourceCode::create(vm, jscSource(stringFromUTF(buffer), SourceOrigin { moduleKey }, WTFMove(moduleURL), TextPosition(), SourceProviderSourceType::Module));
>+    auto sourceCode = JSSourceCode::create(vm, makeSource(stringFromUTF(buffer), SourceOrigin { moduleKey }, WTFMove(moduleURL), TextPosition(), SourceProviderSourceType::Module));
>     catchScope.releaseAssertNoException();
>     auto result = deferred->resolve(exec, sourceCode);
>     catchScope.clearException();
>@@ -1814,7 +1716,7 @@ EncodedJSValue JSC_HOST_CALL functionDollarEvalScript(ExecState* exec)
>         return JSValue::encode(throwException(exec, scope, createError(exec, "Expected global to point to a global object"_s)));
>     
>     NakedPtr<Exception> evaluationException;
>-    JSValue result = evaluate(globalObject->globalExec(), jscSource(sourceCode, exec->callerSourceOrigin()), JSValue(), evaluationException);
>+    JSValue result = evaluate(globalObject->globalExec(), makeSource(sourceCode, exec->callerSourceOrigin()), JSValue(), evaluationException);
>     if (evaluationException)
>         throwException(exec, scope, evaluationException);
>     return JSValue::encode(result);
>@@ -2572,7 +2474,7 @@ static void runWithOptions(GlobalObject* globalObject, CommandLine& options, boo
>         if (isModule) {
>             if (!promise) {
>                 // FIXME: This should use an absolute file URL https://bugs.webkit.org/show_bug.cgi?id=193077
>-                promise = loadAndEvaluateModule(globalObject->globalExec(), jscSource(stringFromUTF(scriptBuffer), SourceOrigin { absolutePath(fileName) }, URL({ }, fileName), TextPosition(), SourceProviderSourceType::Module), jsUndefined());
>+                promise = loadAndEvaluateModule(globalObject->globalExec(), makeSource(stringFromUTF(scriptBuffer), SourceOrigin { absolutePath(fileName) }, URL({ }, fileName), TextPosition(), SourceProviderSourceType::Module), jsUndefined());
>             }
>             scope.clearException();
> 
>diff --git a/Source/JavaScriptCore/parser/SourceProvider.h b/Source/JavaScriptCore/parser/SourceProvider.h
>index bff521cfc0e22b31b4ec0d1add620abbf690fde9..2f0dcf025f327bf906becb5fa745d14477c8ff59 100644
>--- a/Source/JavaScriptCore/parser/SourceProvider.h
>+++ b/Source/JavaScriptCore/parser/SourceProvider.h
>@@ -116,7 +116,6 @@ namespace JSC {
>         virtual unsigned hash() const = 0;
>         virtual StringView source() const = 0;
>         virtual const CachedBytecode* cachedBytecode() const { return nullptr; }
>-        virtual bool cacheBytecode(const CachedBytecode&) const { return false; }
> 
>         StringView getRange(int start, int end) const
>         {
>diff --git a/Source/JavaScriptCore/runtime/CodeCache.cpp b/Source/JavaScriptCore/runtime/CodeCache.cpp
>index 6fc02f8646f8897980338a54313fbe02b8616f2f..2bfb0442485902c5d50d5adb6723b6f57ccb0ea8 100644
>--- a/Source/JavaScriptCore/runtime/CodeCache.cpp
>+++ b/Source/JavaScriptCore/runtime/CodeCache.cpp
>@@ -195,13 +195,30 @@ void generateUnlinkedCodeBlockForFunctions(VM& vm, UnlinkedCodeBlock* unlinkedCo
> 
> void writeCodeBlock(VM& vm, const SourceCodeKey& key, const SourceCodeValue& value)
> {
>+#if OS(DARWIN)
>+    const char* cachePath = Options::diskCachePath();
>+    if (LIKELY(!cachePath))
>+        return;
>+
>     UnlinkedCodeBlock* codeBlock = jsDynamicCast<UnlinkedCodeBlock*>(vm, value.cell.get());
>     if (!codeBlock)
>         return;
> 
>     std::pair<MallocPtr<uint8_t>, size_t> result = encodeCodeBlock(vm, key, codeBlock);
>-    CachedBytecode cachedBytecode { WTFMove(result.first), result.second };
>-    key.source().provider().cacheBytecode(cachedBytecode);
>+
>+    String filename = makeString(cachePath, '/', key.hash(), ".cache");
>+    int fd = open(filename.utf8().data(), O_CREAT | O_WRONLY, 0666);
>+    if (fd == -1)
>+        return;
>+    int rc = flock(fd, LOCK_EX | LOCK_NB);
>+    if (!rc)
>+        ::write(fd, result.first.get(), result.second);
>+    close(fd);
>+#else
>+    UNUSED_PARAM(vm);
>+    UNUSED_PARAM(key);
>+    UNUSED_PARAM(value);
>+#endif
> }
> 
> CachedBytecode serializeBytecode(VM& vm, UnlinkedCodeBlock* codeBlock, const SourceCode& source, SourceCodeType codeType, JSParserStrictMode strictMode, JSParserScriptMode scriptMode, DebuggerMode debuggerMode)
>diff --git a/Source/JavaScriptCore/runtime/CodeCache.h b/Source/JavaScriptCore/runtime/CodeCache.h
>index 2befc63ae2b32cacd973114d9c1fc47199932abc..7ab80fe201abd8bfd92e95f460fc367bb3848b78 100644
>--- a/Source/JavaScriptCore/runtime/CodeCache.h
>+++ b/Source/JavaScriptCore/runtime/CodeCache.h
>@@ -109,14 +109,60 @@ public:
>     template<typename UnlinkedCodeBlockType>
>     UnlinkedCodeBlockType* fetchFromDiskImpl(VM& vm, const SourceCodeKey& key)
>     {
>-        const auto* cachedBytecode = key.source().provider().cachedBytecode();
>-        if (cachedBytecode && cachedBytecode->size()) {
>-            VERBOSE_LOG("Found cached CodeBlock in the SourceProvider");
>-            UnlinkedCodeBlockType* unlinkedCodeBlock = decodeCodeBlock<UnlinkedCodeBlockType>(vm, key, cachedBytecode->data(), cachedBytecode->size());
>-            if (unlinkedCodeBlock)
>-                return unlinkedCodeBlock;
>+        {
>+            const auto* cachedBytecode = key.source().provider().cachedBytecode();
>+            if (cachedBytecode && cachedBytecode->size()) {
>+                VERBOSE_LOG("Found cached CodeBlock in the SourceProvider");
>+                UnlinkedCodeBlockType* unlinkedCodeBlock = decodeCodeBlock<UnlinkedCodeBlockType>(vm, key, cachedBytecode->data(), cachedBytecode->size());
>+                if (unlinkedCodeBlock)
>+                    return unlinkedCodeBlock;
>+            }
>         }
>+
>+#if OS(DARWIN)
>+        const char* cachePath = Options::diskCachePath();
>+        if (!cachePath)
>+            return nullptr;
>+
>+        unsigned hash = key.hash();
>+        char filename[512];
>+        int count = snprintf(filename, 512, "%s/%u.cache", cachePath, hash);
>+        if (count < 0 || count > 512)
>+            return nullptr;
>+
>+        int fd = open(filename, O_RDONLY);
>+        if (fd == -1)
>+            return nullptr;
>+
>+        int rc = flock(fd, LOCK_SH | LOCK_NB);
>+        if (rc) {
>+            close(fd);
>+            return nullptr;
>+        }
>+
>+        struct stat sb;
>+        int res = fstat(fd, &sb);
>+        size_t size = static_cast<size_t>(sb.st_size);
>+        if (res || !size) {
>+            close(fd);
>+            return nullptr;
>+        }
>+
>+        void* buffer = mmap(nullptr, size, PROT_READ, MAP_PRIVATE, fd, 0);
>+        UnlinkedCodeBlockType* unlinkedCodeBlock = decodeCodeBlock<UnlinkedCodeBlockType>(vm, key, buffer, size);
>+        munmap(buffer, size);
>+
>+        if (!unlinkedCodeBlock)
>+            return nullptr;
>+
>+        VERBOSE_LOG("Found cached CodeBlock on disk");
>+        addCache(key, SourceCodeValue(vm, unlinkedCodeBlock, m_age, true));
>+        return unlinkedCodeBlock;
>+#else
>+        UNUSED_PARAM(vm);
>+        UNUSED_PARAM(key);
>         return nullptr;
>+#endif
>     }
> 
>     template<typename UnlinkedCodeBlockType>

Actions: View | Formatted Diff | Diff

Attachments on bug 194762: 362237