Attachment 362374 Details for Bug 194675 – Patch for landing

[patch] Patch for landing

bug-194675-20190219102541.patch (text/plain), 13.11 KB, created by Tadeu Zagallo on 2019-02-19 01:26:19 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-02-19 01:26:19 PST

Size: 13.11 KB

patch

obsolete

>Subversion Revision: 241660
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index eeff7e648620c59aa5ef2982828c5962a794065f..5bcc4df6692707a96a5190cf4bd25b72da92e909 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,32 @@
>+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 Saam Barati.
>+
>+        The code is only used for the bytecode-cache tests, so it should live in
>+        jsc.cpp rather than in the CodeCache. The logic now lives in ShellSourceProvider,
>+        which overrides the a virtual method in SourceProvider, `cacheBytecode`,
>+        in order to write the cache to disk.
>+
>+        * jsc.cpp:
>+        (ShellSourceProvider::create):
>+        (ShellSourceProvider::~ShellSourceProvider):
>+        (ShellSourceProvider::cachePath const):
>+        (ShellSourceProvider::loadBytecode):
>+        (ShellSourceProvider::ShellSourceProvider):
>+        (jscSource):
>+        (GlobalObject::moduleLoaderFetch):
>+        (functionDollarEvalScript):
>+        (runWithOptions):
>+        * parser/SourceProvider.h:
>+        (JSC::SourceProvider::cacheBytecode const):
>+        * runtime/CodeCache.cpp:
>+        (JSC::writeCodeBlock):
>+        * runtime/CodeCache.h:
>+        (JSC::CodeCacheMap::fetchFromDiskImpl):
>+
> 2019-02-18  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         Add version number to cached bytecode
>diff --git a/Source/JavaScriptCore/jsc.cpp b/Source/JavaScriptCore/jsc.cpp
>index be0180605e6e8abf48ab711b86c9d4fc98c05f1a..75751426e412f5239450a13e3d9cf865fd35b279 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,100 @@ 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;
>+    }
>+
>+    void cacheBytecode(const BytecodeCacheGenerator& generator) const override
>+    {
>+#if OS(DARWIN)
>+        String filename = cachePath();
>+        if (filename.isNull())
>+            return;
>+        int fd = open(filename.utf8().data(), O_CREAT | O_WRONLY | O_TRUNC | O_EXLOCK | O_NONBLOCK, 0666);
>+        if (fd == -1)
>+            return;
>+        CachedBytecode cachedBytecode = generator();
>+        write(fd, cachedBytecode.data(), cachedBytecode.size());
>+        close(fd);
>+#endif
>+    }
>+
>+private:
>+    String cachePath() const
>+    {
>+        const char* cachePath = Options::diskCachePath();
>+        if (!cachePath)
>+            return 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 | 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 +1131,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 +1804,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 +2560,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..2493d755e338d65ea43b1400bf2dc2162a43ac37 100644
>--- a/Source/JavaScriptCore/parser/SourceProvider.h
>+++ b/Source/JavaScriptCore/parser/SourceProvider.h
>@@ -104,6 +104,7 @@ namespace JSC {
>         const void* m_data;
>     };
> 
>+    using BytecodeCacheGenerator = Function<CachedBytecode()>;
> 
>     class SourceProvider : public RefCounted<SourceProvider> {
>     public:
>@@ -116,6 +117,7 @@ namespace JSC {
>         virtual unsigned hash() const = 0;
>         virtual StringView source() const = 0;
>         virtual const CachedBytecode* cachedBytecode() const { return nullptr; }
>+        virtual void cacheBytecode(const BytecodeCacheGenerator&) const { }
> 
>         StringView getRange(int start, int end) const
>         {
>diff --git a/Source/JavaScriptCore/runtime/CodeCache.cpp b/Source/JavaScriptCore/runtime/CodeCache.cpp
>index 2bfb0442485902c5d50d5adb6723b6f57ccb0ea8..fd5f9687616f7145013ac28706bd5f714668a805 100644
>--- a/Source/JavaScriptCore/runtime/CodeCache.cpp
>+++ b/Source/JavaScriptCore/runtime/CodeCache.cpp
>@@ -195,30 +195,14 @@ 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);
>-
>-    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
>+    key.source().provider().cacheBytecode([&] {
>+        std::pair<MallocPtr<uint8_t>, size_t> result = encodeCodeBlock(vm, key, codeBlock);
>+        return CachedBytecode { WTFMove(result.first), result.second };
>+    });
> }
> 
> 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 0e41e0211cad9c5db3fcd88adca6433e4eb78e2e..f14c27b4e491a334f830b34607ded3d6ddcd4782 100644
>--- a/Source/JavaScriptCore/runtime/CodeCache.h
>+++ b/Source/JavaScriptCore/runtime/CodeCache.h
>@@ -39,10 +39,6 @@
> #include "UnlinkedFunctionCodeBlock.h"
> #include "UnlinkedModuleProgramCodeBlock.h"
> #include "UnlinkedProgramCodeBlock.h"
>-#include <sys/stat.h>
>-#include <wtf/Forward.h>
>-#include <wtf/Scope.h>
>-#include <wtf/text/WTFString.h>
> 
> namespace JSC {
> 
>@@ -110,60 +106,14 @@ 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 CachedBytecode* 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;
>-
>-        auto closeFD = makeScopeExit([&] {
>-            close(fd);
>-        });
>-
>-        int rc = flock(fd, LOCK_SH | LOCK_NB);
>-        if (rc)
>-            return nullptr;
>-
>-        struct stat sb;
>-        int res = fstat(fd, &sb);
>-        size_t size = static_cast<size_t>(sb.st_size);
>-        if (res || !size)
>-            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 194675: 362064 | 362108 | 362111 | 362271 | 362281 | 362322 | 362374