JavaScriptCore:
authormjs@apple.com <mjs@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Sep 2008 12:17:33 +0000 (12:17 +0000)
committermjs@apple.com <mjs@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Sep 2008 12:17:33 +0000 (12:17 +0000)
2008-09-08  Maciej Stachowiak  <mjs@apple.com>

        Reviewed by Anders Carlsson.

        - Cache the code generated for eval to speed up SunSpider and web sites
        https://bugs.webkit.org/show_bug.cgi?id=20718

        1.052x on SunSpider
        2.29x on date-format-tofte

        Lots of real sites seem to get many hits on this cache as well,
        including GMail, Google Spreadsheets, Slate and Digg (the last of
        these gets over 100 hits on initial page load).

        * VM/CodeBlock.h:
        (JSC::EvalCodeCache::get):
        * VM/Machine.cpp:
        (JSC::Machine::callEval):
        (JSC::Machine::privateExecute):
        (JSC::Machine::cti_op_call_eval):
        * VM/Machine.h:

LayoutTests:

2008-09-08  Maciej Stachowiak  <mjs@apple.com>

        Reviewed by Anders Carlsson.

        - Test for potential bug found while fixing "Cache the code generated for eval to speed up SunSpider and web sites"
        https://bugs.webkit.org/show_bug.cgi?id=20718

        * fast/js/eval-cache-crash-expected.txt: Added.
        * fast/js/eval-cache-crash.html: Added.
        * fast/js/resources/eval-cache-crash.js: Added.

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

JavaScriptCore/ChangeLog
JavaScriptCore/VM/CodeBlock.h
JavaScriptCore/VM/Machine.cpp
JavaScriptCore/VM/Machine.h
LayoutTests/ChangeLog
LayoutTests/fast/js/eval-cache-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/js/eval-cache-crash.html [new file with mode: 0644]
LayoutTests/fast/js/resources/eval-cache-crash.js [new file with mode: 0644]

index 0abc557..6b2e791 100644 (file)
@@ -1,3 +1,25 @@
+2008-09-08  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Anders Carlsson.
+        
+        - Cache the code generated for eval to speed up SunSpider and web sites
+        https://bugs.webkit.org/show_bug.cgi?id=20718
+        
+        1.052x on SunSpider
+        2.29x on date-format-tofte
+        
+        Lots of real sites seem to get many hits on this cache as well,
+        including GMail, Google Spreadsheets, Slate and Digg (the last of
+        these gets over 100 hits on initial page load).
+
+        * VM/CodeBlock.h:
+        (JSC::EvalCodeCache::get):
+        * VM/Machine.cpp:
+        (JSC::Machine::callEval):
+        (JSC::Machine::privateExecute):
+        (JSC::Machine::cti_op_call_eval):
+        * VM/Machine.h:
+
 2008-09-07  Cameron Zwarich  <cwzwarich@uwaterloo.ca>
 
         Reviewed by Oliver Hunt.
index a1c29dd..991ea61 100644 (file)
@@ -33,6 +33,7 @@
 #include "Instruction.h"
 #include "JSGlobalObject.h"
 #include "nodes.h"
+#include "Parser.h"
 #include "SourceRange.h"
 #include "ustring.h"
 #include <wtf/RefPtr.h>
@@ -129,6 +130,40 @@ namespace JSC {
 #endif
     };
 
+    class EvalCodeCache {
+    public:
+        PassRefPtr<EvalNode> get(ExecState* exec, const UString& evalSource, ScopeChainNode* scopeChain, JSValue*& exceptionValue)
+        {
+            RefPtr<EvalNode> evalNode;
+
+            if (evalSource.size() < maxCacheableSourceLength && (*scopeChain->begin())->isVariableObject())
+                evalNode = cacheMap.get(evalSource.rep());
+
+            if (!evalNode) {
+                int sourceId;
+                int errLine;
+                UString errMsg;
+                
+                evalNode = exec->parser()->parse<EvalNode>(exec, UString(), 1, UStringSourceProvider::create(evalSource), &sourceId, &errLine, &errMsg);
+                if (evalNode) {
+                    if (evalSource.size() < maxCacheableSourceLength && (*scopeChain->begin())->isVariableObject() && cacheMap.size() < maxCacheEntries)
+                        cacheMap.set(evalSource.rep(), evalNode);
+                } else {
+                    exceptionValue = Error::create(exec, SyntaxError, errMsg, errLine, sourceId, NULL);
+                    return 0;
+                }
+            }
+
+            return evalNode.release();
+        }
+
+    private:
+        static const int maxCacheableSourceLength = 256;
+        static const int maxCacheEntries = 64;
+
+        HashMap<RefPtr<UString::Rep>, RefPtr<EvalNode> > cacheMap;
+    };
+
     struct CodeBlock {
         CodeBlock(ScopeNode* ownerNode_, CodeType codeType_, PassRefPtr<SourceProvider> source_, unsigned sourceOffset_)
             : ownerNode(ownerNode_)
@@ -207,10 +242,13 @@ namespace JSC {
         HashMap<void*, unsigned> ctiReturnAddressVPCMap;
 #endif
 
+        EvalCodeCache evalCodeCache;
+
     private:
 #if !defined(NDEBUG) || ENABLE(SAMPLING_TOOL)
         void dump(ExecState*, const Vector<Instruction>::const_iterator& begin, Vector<Instruction>::const_iterator&) const;
 #endif
+
     };
 
     // Program code is not marked by any function, so we make the global object
index cda9d06..8001104 100644 (file)
@@ -493,7 +493,7 @@ static NEVER_INLINE bool isNotObject(ExecState* exec, bool forInstanceOf, CodeBl
     return true;
 }
 
-NEVER_INLINE JSValue* Machine::callEval(ExecState* exec, JSObject* thisObj, ScopeChainNode* scopeChain, RegisterFile* registerFile, Register* r, int argv, int argc, JSValue*& exceptionValue)
+NEVER_INLINE JSValue* Machine::callEval(ExecState* exec, CodeBlock* callingCodeBlock, JSObject* thisObj, ScopeChainNode* scopeChain, RegisterFile* registerFile, Register* r, int argv, int argc, JSValue*& exceptionValue)
 {
     if (argc < 2)
         return jsUndefined();
@@ -507,19 +507,13 @@ NEVER_INLINE JSValue* Machine::callEval(ExecState* exec, JSObject* thisObj, Scop
     if (*profiler)
         (*profiler)->willExecute(exec, scopeChain->globalObject()->evalFunction());
 
-    int sourceId;
-    int errLine;
-    UString errMsg;
-    RefPtr<EvalNode> evalNode = exec->parser()->parse<EvalNode>(exec, UString(), 1, UStringSourceProvider::create(static_cast<JSString*>(program)->value()), &sourceId, &errLine, &errMsg);
+    UString programSource = static_cast<JSString*>(program)->value();
 
-    if (!evalNode) {
-        exceptionValue = Error::create(exec, SyntaxError, errMsg, errLine, sourceId, NULL);
-        if (*profiler)
-            (*profiler)->didExecute(exec, scopeChain->globalObject()->evalFunction());
-        return 0;
-    }
+    RefPtr<EvalNode> evalNode = callingCodeBlock->evalCodeCache.get(exec, programSource, scopeChain, exceptionValue);
 
-    JSValue* result = exec->globalData().machine->execute(evalNode.get(), exec, thisObj, r - registerFile->base() + argv + argc, scopeChain, &exceptionValue);
+    JSValue* result = 0;
+    if (evalNode)
+        result = exec->globalData().machine->execute(evalNode.get(), exec, thisObj, r - registerFile->base() + argv + argc, scopeChain, &exceptionValue);
 
     if (*profiler)
         (*profiler)->didExecute(exec, scopeChain->globalObject()->evalFunction());
@@ -2880,7 +2874,7 @@ JSValue* Machine::privateExecute(ExecutionFlag flag, ExecState* exec, RegisterFi
 
         if (baseVal == scopeChain->globalObject() && funcVal == scopeChain->globalObject()->evalFunction()) {
             JSObject* thisObject = static_cast<JSObject*>(r[codeBlock->thisRegister].jsValue(exec));
-            JSValue* result = callEval(exec, thisObject, scopeChain, registerFile, r, firstArg, argCount, exceptionValue);
+            JSValue* result = callEval(exec, codeBlock, thisObject, scopeChain, registerFile, r, firstArg, argCount, exceptionValue);
             if (exceptionValue)
                 goto vm_throw;
 
@@ -4781,7 +4775,7 @@ JSValue* Machine::cti_op_call_eval(CTI_ARGS)
 
     if (baseVal == scopeChain->globalObject() && funcVal == scopeChain->globalObject()->evalFunction()) {
         JSObject* thisObject = static_cast<JSObject*>(r[codeBlock->thisRegister].jsValue(exec));
-        JSValue* result = machine->callEval(exec, thisObject, scopeChain, registerFile,  r, firstArg, argCount, exceptionValue);
+        JSValue* result = machine->callEval(exec, codeBlock, thisObject, scopeChain, registerFile,  r, firstArg, argCount, exceptionValue);
         JSVALUE_VM_CHECK_EXCEPTION_ARG(exceptionValue);
         return result;
     }
index b40c5ea..13475d3 100644 (file)
@@ -229,7 +229,7 @@ namespace JSC {
     private:
         enum ExecutionFlag { Normal, InitializeAndReturn };
 
-        NEVER_INLINE JSValue* callEval(ExecState* exec, JSObject* thisObj, ScopeChainNode* scopeChain, RegisterFile*, Register* r, int argv, int argc, JSValue*& exceptionValue);
+        NEVER_INLINE JSValue* callEval(ExecState* exec, CodeBlock* callingCodeBlock, JSObject* thisObj, ScopeChainNode* scopeChain, RegisterFile*, Register* r, int argv, int argc, JSValue*& exceptionValue);
         JSValue* execute(EvalNode*, ExecState*, JSObject* thisObj, int registerOffset, ScopeChainNode*, JSValue** exception);
 
         ALWAYS_INLINE void initializeCallFrame(Register* callFrame, CodeBlock*, Instruction*, ScopeChainNode*, Register* r, int returnValueRegister, int argv, int argc, int calledAsConstructor, JSValue* function);
index 22bd3fb..2f3d5e9 100644 (file)
@@ -1,3 +1,14 @@
+2008-09-08  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Anders Carlsson.
+
+        - Test for potential bug found while fixing "Cache the code generated for eval to speed up SunSpider and web sites"
+        https://bugs.webkit.org/show_bug.cgi?id=20718
+
+        * fast/js/eval-cache-crash-expected.txt: Added.
+        * fast/js/eval-cache-crash.html: Added.
+        * fast/js/resources/eval-cache-crash.js: Added.
+
 2008-09-07  Adam Barth  <abarth@webkit.org>
 
         Reviewed by Sam Weinig.
diff --git a/LayoutTests/fast/js/eval-cache-crash-expected.txt b/LayoutTests/fast/js/eval-cache-crash-expected.txt
new file mode 100644 (file)
index 0000000..b2e7c74
--- /dev/null
@@ -0,0 +1,11 @@
+Test to make sure the eval code cache doesn't crash or give wrong results in odd situations.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS first is 'first'
+PASS second is 'second'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/js/eval-cache-crash.html b/LayoutTests/fast/js/eval-cache-crash.html
new file mode 100644 (file)
index 0000000..77bbd9a
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="resources/js-test-style.css">
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="resources/eval-cache-crash.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/js/resources/eval-cache-crash.js b/LayoutTests/fast/js/resources/eval-cache-crash.js
new file mode 100644 (file)
index 0000000..8e68f9b
--- /dev/null
@@ -0,0 +1,17 @@
+description(
+"Test to make sure the eval code cache doesn't crash or give wrong results in odd situations."
+);
+
+
+var str = "(function () { return a; })";
+var a = "first";
+var first = eval(str)();
+shouldBe("first", "'first'");
+
+with ({a : "second"}) {
+    var second = eval(str)();
+}
+
+shouldBe("second", "'second'");
+
+var successfullyParsed = true;
\ No newline at end of file