Attachment 362108 Details for Bug 194675 – Patch

[patch] Patch

bug-194675-20190215120646.patch (text/plain), 12.26 KB, created by Tadeu Zagallo on 2019-02-15 03:07:19 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-02-15 03:07:19 PST

Size: 12.26 KB

patch

obsolete

>Subversion Revision: 241550
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 4306f8ec7c8e733b428bebc21898477b23f53164..ba07312b6767117e92438972b95c0ebb5fa89205 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2019-02-14  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:
>+        (CliSourceProvider::create):
>+        (CliSourceProvider::~CliSourceProvider):
>+        (CliSourceProvider::cachePath const):
>+        (CliSourceProvider::loadBytecode):
>+        (CliSourceProvider::CliSourceProvider):
>+        (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-14  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         CachedBitVector's size must be converted from bits to bytes
>diff --git a/Source/JavaScriptCore/jsc.cpp b/Source/JavaScriptCore/jsc.cpp
>index be0180605e6e8abf48ab711b86c9d4fc98c05f1a..0d61cc06c5737f5440aa9a76bcf7f2feb53eb68f 100644
>--- a/Source/JavaScriptCore/jsc.cpp
>+++ b/Source/JavaScriptCore/jsc.cpp
>@@ -954,12 +954,108 @@ 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 (m_cachedBytecode.size())
>+            munmap(const_cast<void*>(m_cachedBytecode.data()), m_cachedBytecode.size());
>+    }
>+
>+    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 makeSource(str, sourceOrigin, URL({ }, filename));
>+    return jscSource(str, sourceOrigin, URL({ }, filename));
> }
> 
> template<typename Vector>
>@@ -1042,7 +1138,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 +1811,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 +2567,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 3778bf941ad7fac864d6ca780a06d8289e91ae4c..b3c9c465764fffcb9705ce86fb9ca804c2148645 100644
>--- a/Source/JavaScriptCore/parser/SourceProvider.h
>+++ b/Source/JavaScriptCore/parser/SourceProvider.h
>@@ -116,6 +116,7 @@ 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 c1ecedbf7f91a346354d90b396c0bcce525af60b..cd0c3e0cbc43bbeb9c2fd4b7454b9e4ba8c7b970 100644
>--- a/Source/JavaScriptCore/runtime/CodeCache.cpp
>+++ b/Source/JavaScriptCore/runtime/CodeCache.cpp
>@@ -192,30 +192,13 @@ 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
>+    CachedBytecode cachedBytecode { WTFMove(result.first), result.second };
>+    key.source().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 be44bddee99fe494e1885ca3e7791da6694f889d..45938e6f62887c2b2aa961fbdc202516c7f8dc10 100644
>--- a/Source/JavaScriptCore/runtime/CodeCache.h
>+++ b/Source/JavaScriptCore/runtime/CodeCache.h
>@@ -109,60 +109,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 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 194675: 362064 | 362108 | 362111 | 362271 | 362281 | 362322 | 362374