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: http://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 0abc55795cf2b82621ece6f77d649741ac9dcdc9..6b2e7911b835a999c25b351f6158e618036cf416 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 a1c29dd3c634bd1a307f9a379ff05c8644ea73ed..991ea613ea603d9ffeba43f8ce34fd2c3d168a7e 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 cda9d0611761731254954ed628ed4f92f357ee12..80011047de7b42a296f6be38902d5f8030a607a4 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 b40c5ea4ff5dd2c35be7ba1f29ee89e4f9dc88b0..13475d30cf90492af82049f89c1feaaa5366f2e0 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 22bd3fb8aba57ab03da0fb0bc406ea510fac3383..2f3d5e97ab9f7c7af050acbdd720bc31bdfbf791 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