Unreviewed, rolling out r241612.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 17 Feb 2019 18:13:23 +0000 (18:13 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 17 Feb 2019 18:13:23 +0000 (18:13 +0000)
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

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241651 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/jsc.cpp
Source/JavaScriptCore/parser/SourceProvider.h
Source/JavaScriptCore/runtime/CodeCache.cpp
Source/JavaScriptCore/runtime/CodeCache.h

index 4a261f1..165c182 100644 (file)
@@ -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
index 37ff1d7..2da8f91 100644 (file)
@@ -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();
 
index bff521c..2f0dcf0 100644 (file)
@@ -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
         {
index 6fc02f8..2bfb044 100644 (file)
@@ -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)
index 2befc63..7ab80fe 100644 (file)
@@ -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>