Attachment 362271 Details for Bug 194675 – Patch

[patch] Patch

bug-194675-20190218103845.patch (text/plain), 13.07 KB, created by Tadeu Zagallo on 2019-02-18 01:39:21 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-02-18 01:39:21 PST

Size: 13.07 KB

patch

obsolete

>Subversion Revision: 241658
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 13f1d17d290f781a0f252eb496b6448f177f474e..5b77666999e1173a0f5eb26965fb212f08a3a3b4 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,31 @@
>+2019-02-18  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        Move bytecode cache-related filesystem code out of CodeCache
>+        https://bugs.webkit.org/show_bug.cgi?id=194675
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        That code is only used for the bytecode-cache tests, so it should live in
>+        jsc.cpp rather than in the CodeCache.
>+
>+        * jsc.cpp:
>+        (ShellSourceProvider::create):
>+        (ShellSourceProvider::~ShellSourceProvider):
>+        (ShellSourceProvider::cachePath const):
>+        (ShellSourceProvider::loadBytecode):
>+        (ShellSourceProvider::ShellSourceProvider):
>+        (jscSource):
>+        (GlobalObject::moduleLoaderFetch):
>+        (functionDollarEvalScript):
>+        (runWithOptions):
>+        * parser/SourceProvider.h:
>+        (JSC::SourceProvider::useBytecodeCache const):
>+        (JSC::SourceProvider::cacheBytecode const):
>+        * runtime/CodeCache.cpp:
>+        (JSC::writeCodeBlock):
>+        * runtime/CodeCache.h:
>+        (JSC::CodeCacheMap::fetchFromDiskImpl):
>+
> 2019-02-17  Saam Barati  <sbarati@apple.com>
> 
>         WasmB3IRGenerator models some effects incorrectly
>diff --git a/Source/JavaScriptCore/jsc.cpp b/Source/JavaScriptCore/jsc.cpp
>index be0180605e6e8abf48ab711b86c9d4fc98c05f1a..5e121e67f15989e1ae78bed8ee6caab21aba189f 100644
>--- a/Source/JavaScriptCore/jsc.cpp
>+++ b/Source/JavaScriptCore/jsc.cpp
>@@ -87,6 +87,7 @@
> #include <wtf/MemoryPressureHandler.h>
> #include <wtf/MonotonicTime.h>
> #include <wtf/NeverDestroyed.h>
>+#include <wtf/Scope.h>
> #include <wtf/StringPrintStream.h>
> #include <wtf/URL.h>
> #include <wtf/WallTime.h>
>@@ -954,12 +955,106 @@ 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
>+    }
>+
>+    bool useBytecodeCache() const override
>+    {
>+        return !!Options::diskCachePath();
>+    }
>+
>+    const CachedBytecode* cachedBytecode() const override
>+    {
>+        ASSERT(useBytecodeCache());
>+        return &m_cachedBytecode;
>+    }
>+
>+    void cacheBytecode(const CachedBytecode& cachedBytecode) const override
>+    {
>+        ASSERT(useBytecodeCache());
>+#if OS(DARWIN)
>+        String filename = cachePath();
>+        int fd = open(filename.utf8().data(), O_CREAT | O_WRONLY | O_TRUNC | O_EXLOCK | O_NONBLOCK, 0666);
>+        if (fd == -1)
>+            return;
>+        write(fd, cachedBytecode.data(), cachedBytecode.size());
>+        close(fd);
>+#endif
>+    }
>+
>+private:
>+    String cachePath() const
>+    {
>+        const char* cachePath = Options::diskCachePath();
>+        ASSERT(cachePath);
>+        String filename = sourceOrigin().string();
>+        filename.replace('/', '_');
>+        return makeString(cachePath, '/', source().toString().hash(), '-', filename, ".bytecode-cache");
>+    }
>+
>+    void loadBytecode()
>+    {
>+#if OS(DARWIN)
>+        if (!useBytecodeCache())
>+            return;
>+
>+        String filename = cachePath();
>+        if (filename.isNull())
>+            return;
>+
>+        int fd = open(filename.utf8().data(), O_RDONLY | O_SHLOCK | O_NONBLOCK);
>+        if (fd == -1)
>+            return;
>+
>+        auto closeFD = makeScopeExit([&] {
>+            close(fd);
>+        });
>+
>+        struct stat sb;
>+        int res = fstat(fd, &sb);
>+        size_t size = static_cast<size_t>(sb.st_size);
>+        if (res || !size)
>+            return;
>+
>+        void* buffer = mmap(nullptr, size, PROT_READ, MAP_PRIVATE, fd, 0);
>+        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 makeSource(str, sourceOrigin, URL({ }, filename));
>+    return jscSource(str, sourceOrigin, URL({ }, filename));
> }
> 
> template<typename Vector>
>@@ -1042,7 +1137,7 @@ JSInternalPromise* GlobalObject::moduleLoaderFetch(JSGlobalObject* globalObject,
>     }
> #endif
> 
>-    auto sourceCode = JSSourceCode::create(vm, makeSource(stringFromUTF(buffer), SourceOrigin { moduleKey }, WTFMove(moduleURL), TextPosition(), SourceProviderSourceType::Module));
>+    auto sourceCode = JSSourceCode::create(vm, jscSource(stringFromUTF(buffer), SourceOrigin { moduleKey }, WTFMove(moduleURL), TextPosition(), SourceProviderSourceType::Module));
>     catchScope.releaseAssertNoException();
>     auto result = deferred->resolve(exec, sourceCode);
>     catchScope.clearException();
>@@ -1715,7 +1810,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(), makeSource(sourceCode, exec->callerSourceOrigin()), JSValue(), evaluationException);
>+    JSValue result = evaluate(globalObject->globalExec(), jscSource(sourceCode, exec->callerSourceOrigin()), JSValue(), evaluationException);
>     if (evaluationException)
>         throwException(exec, scope, evaluationException);
>     return JSValue::encode(result);
>@@ -2471,7 +2566,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(), makeSource(stringFromUTF(scriptBuffer), SourceOrigin { absolutePath(fileName) }, URL({ }, fileName), TextPosition(), SourceProviderSourceType::Module), jsUndefined());
>+                promise = loadAndEvaluateModule(globalObject->globalExec(), jscSource(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 2f0dcf025f327bf906becb5fa745d14477c8ff59..85210abf4cf97ac93c55435dc6003e8b880d4486 100644
>--- a/Source/JavaScriptCore/parser/SourceProvider.h
>+++ b/Source/JavaScriptCore/parser/SourceProvider.h
>@@ -115,7 +115,9 @@ namespace JSC {
> 
>         virtual unsigned hash() const = 0;
>         virtual StringView source() const = 0;
>+        virtual bool useBytecodeCache() const { return false; }
>         virtual const CachedBytecode* cachedBytecode() const { return nullptr; }
>+        virtual void cacheBytecode(const CachedBytecode&) const { }
> 
>         StringView getRange(int start, int end) const
>         {
>diff --git a/Source/JavaScriptCore/runtime/CodeCache.cpp b/Source/JavaScriptCore/runtime/CodeCache.cpp
>index 2bfb0442485902c5d50d5adb6723b6f57ccb0ea8..2718ec56feae8dd439e907afcf37648112f901cb 100644
>--- a/Source/JavaScriptCore/runtime/CodeCache.cpp
>+++ b/Source/JavaScriptCore/runtime/CodeCache.cpp
>@@ -195,9 +195,8 @@ 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))
>+    const SourceProvider& provider = key.source().provider();
>+    if (!provider.useBytecodeCache())
>         return;
> 
>     UnlinkedCodeBlock* codeBlock = jsDynamicCast<UnlinkedCodeBlock*>(vm, value.cell.get());
>@@ -205,20 +204,8 @@ void writeCodeBlock(VM& vm, const SourceCodeKey& key, const SourceCodeValue& val
>         return;
> 
>     std::pair<MallocPtr<uint8_t>, size_t> result = encodeCodeBlock(vm, key, codeBlock);
>-
>-    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 cachedBytecode { WTFMove(result.first), result.second };
>+    provider.cacheBytecode(cachedBytecode);
> }
> 
> 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 7ab80fe201abd8bfd92e95f460fc367bb3848b78..747b867f1d0607dbc704fe7de78563f128248908 100644
>--- a/Source/JavaScriptCore/runtime/CodeCache.h
>+++ b/Source/JavaScriptCore/runtime/CodeCache.h
>@@ -39,9 +39,6 @@
> #include "UnlinkedFunctionCodeBlock.h"
> #include "UnlinkedModuleProgramCodeBlock.h"
> #include "UnlinkedProgramCodeBlock.h"
>-#include <sys/stat.h>
>-#include <wtf/Forward.h>
>-#include <wtf/text/WTFString.h>
> 
> namespace JSC {
> 
>@@ -109,60 +106,18 @@ 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;
>-            }
>-        }
>-
>-#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);
>+        const SourceProvider& provider = key.source().provider();
>+        if (!provider.useBytecodeCache())
>             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;
>+        const CachedBytecode* cachedBytecode = 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;
>         }
>-
>-        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 194675: 362064 | 362108 | 362111 | 362271 | 362281 | 362322 | 362374