Bug 767750 - rm JSScript::evalHashLink (r=njn)
authorLuke Wagner <luke@mozilla.com>
Tue, 03 Jul 2012 10:24:35 -0700
changeset 99092 ebc800948e7a3137c486974b1a350ab5b1eca916
parent 99091 d9650bc4da1a12cf3b913ab09a61611f953ce5b1
child 99093 7221c50cb5b43f34c0aab6af24aef4c9b65d080a
push id23102
push userryanvm@gmail.com
push dateFri, 13 Jul 2012 00:46:37 +0000
treeherdermozilla-central@6489be1890c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs767750
milestone16.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 767750 - rm JSScript::evalHashLink (r=njn)
js/src/builtin/Eval.cpp
js/src/jsapi.cpp
js/src/jscntxt.h
js/src/jsgc.cpp
js/src/jsscript.h
--- a/js/src/builtin/Eval.cpp
+++ b/js/src/builtin/Eval.cpp
@@ -5,16 +5,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "jscntxt.h"
 #include "jsonparser.h"
 
 #include "builtin/Eval.h"
 #include "frontend/BytecodeCompiler.h"
+#include "mozilla/HashFunctions.h"
 #include "vm/GlobalObject.h"
 
 #include "jsinterpinlines.h"
 
 using namespace js;
 
 // We should be able to assert this for *any* fp->scopeChain().
 static void
@@ -25,160 +26,100 @@ AssertInnerizedScopeChain(JSContext *cx,
         if (JSObjectOp op = o->getClass()->ext.innerObject) {
             Rooted<JSObject*> obj(cx, o);
             JS_ASSERT(op(cx, obj) == o);
         }
     }
 #endif
 }
 
-void
-EvalCache::purge()
+static bool
+IsEvalCacheCandidate(JSScript *script)
 {
-    // Purge all scripts from the eval cache. In addition to removing them from
-    // table_, null out the evalHashLink field of any script removed. Since
-    // evalHashLink is in a union with globalObject, this allows the GC to
-    // indiscriminately use the union as a nullable globalObject pointer.
-    for (size_t i = 0; i < ArrayLength(table_); ++i) {
-        for (JSScript **listHeadp = &table_[i]; *listHeadp; ) {
-            JSScript *script = *listHeadp;
-            JS_ASSERT(GetGCThingTraceKind(script) == JSTRACE_SCRIPT);
-            *listHeadp = script->evalHashLink;
-            script->evalHashLink = NULL;
-        }
-    }
-}
-
-JSScript **
-EvalCache::bucket(JSLinearString *str)
-{
-    const jschar *s = str->chars();
-    size_t n = str->length();
-
-    if (n > 100)
-        n = 100;
-    uint32_t h;
-    for (h = 0; n; s++, n--)
-        h = JS_ROTATE_LEFT32(h, 4) ^ *s;
-
-    h *= JS_GOLDEN_RATIO;
-    h >>= 32 - SHIFT;
-    JS_ASSERT(h < ArrayLength(table_));
-    return &table_[h];
+    // Make sure there are no inner objects which might use the wrong parent
+    // and/or call scope by reusing the previous eval's script. Skip the
+    // script's first object, which entrains the eval's scope.
+    return script->savedCallerFun &&
+           !script->hasSingletons &&
+           script->objects()->length == 1 &&
+           !script->hasRegexps();
 }
 
-static JSScript *
-EvalCacheLookup(JSContext *cx, JSLinearString *str, StackFrame *caller, unsigned staticLevel,
-                JSPrincipals *principals, JSObject &scopeobj, JSScript **bucket)
+/* static */ HashNumber
+EvalCacheHashPolicy::hash(const EvalCacheLookup &l)
 {
-    // Cache local eval scripts indexed by source qualified by scope.
-    // 
-    // An eval cache entry should never be considered a hit unless its
-    // strictness matches that of the new eval code. The existing code takes
-    // care of this, because hits are qualified by the function from which
-    // eval was called, whose strictness doesn't change. (We don't cache evals
-    // in eval code, so the calling function corresponds to the calling script,
-    // and its strictness never varies.) Scripts produced by calls to eval from
-    // global code aren't cached.
-    // 
-    // FIXME bug 620141: Qualify hits by calling script rather than function.
-    // Then we wouldn't need the unintuitive !isEvalFrame() hack in EvalKernel
-    // to avoid caching nested evals in functions (thus potentially mismatching
-    // on strict mode), and we could cache evals in global code if desired.
-    unsigned count = 0;
-    JSScript **scriptp = bucket;
+    return AddToHash(HashString(l.str->chars(), l.str->length()),
+                     l.caller,
+                     l.staticLevel,
+                     l.version,
+                     l.compartment);
+}
 
-    JSVersion version = cx->findVersion();
-    JSScript *script;
-    JSSubsumePrincipalsOp subsume = cx->runtime->securityCallbacks->subsumePrincipals;
-    while ((script = *scriptp) != NULL) {
-        if (script->savedCallerFun &&
-            script->staticLevel == staticLevel &&
-            script->getVersion() == version &&
-            !script->hasSingletons &&
-            (!subsume || script->principals == principals ||
-             (subsume(principals, script->principals) &&
-              subsume(script->principals, principals))))
-        {
-            // Get the prior (cache-filling) eval's saved caller function.
-            // See frontend::CompileScript.
-            JSFunction *fun = script->getCallerFunction();
-
-            if (fun == caller->fun()) {
-                /*
-                 * Get the source string passed for safekeeping in the atom map
-                 * by the prior eval to frontend::CompileScript.
-                 */
-                JSAtom *src = script->atoms[0];
+/* static */ bool
+EvalCacheHashPolicy::match(JSScript *script, const EvalCacheLookup &l)
+{
+    JS_ASSERT(IsEvalCacheCandidate(script));
 
-                if (src == str || EqualStrings(src, str)) {
-                    // Source matches. Make sure there are no inner objects
-                    // which might use the wrong parent and/or call scope by
-                    // reusing the previous eval's script. Skip the script's
-                    // first object, which entrains the eval's scope.
-                    JS_ASSERT(script->objects()->length >= 1);
-                    if (script->objects()->length == 1 &&
-                        !script->hasRegexps()) {
-                        JS_ASSERT(staticLevel == script->staticLevel);
-                        *scriptp = script->evalHashLink;
-                        script->evalHashLink = NULL;
-                        return script;
-                    }
-                }
-            }
-        }
+    // Get the source string passed for safekeeping in the atom map
+    // by the prior eval to frontend::CompileScript.
+    JSAtom *keyStr = script->atoms[0];
 
-        static const unsigned EVAL_CACHE_CHAIN_LIMIT = 4;
-        if (++count == EVAL_CACHE_CHAIN_LIMIT)
-            return NULL;
-        scriptp = script->evalHashLink.unsafeGet();
-    }
-    return NULL;
+    return EqualStrings(keyStr, l.str) &&
+           script->getCallerFunction() == l.caller &&
+           script->staticLevel == l.staticLevel &&
+           script->getVersion() == l.version &&
+           script->compartment() == l.compartment;
 }
 
 // There are two things we want to do with each script executed in EvalKernel:
 //  1. notify jsdbgapi about script creation/destruction
 //  2. add the script to the eval cache when EvalKernel is finished
 //
 // NB: Although the eval cache keeps a script alive wrt to the JS engine, from
 // a jsdbgapi user's perspective, we want each eval() to create and destroy a
 // script. This hides implementation details and means we don't have to deal
-// with calls to JS_GetScriptObject for scripts in the eval cache (currently,
-// script->object aliases script->evalHashLink()).
+// with calls to JS_GetScriptObject for scripts in the eval cache.
 class EvalScriptGuard
 {
     JSContext *cx_;
-    JSLinearString *str_;
-    JSScript **bucket_;
     Rooted<JSScript*> script_;
 
+    /* These fields are only valid if lookup_.str is non-NULL. */
+    EvalCacheLookup lookup_;
+    EvalCache::AddPtr p_;
+
   public:
-    EvalScriptGuard(JSContext *cx, JSLinearString *str)
-      : cx_(cx),
-        str_(str),
-        script_(cx) {
-        bucket_ = cx->runtime->evalCache.bucket(str);
+    EvalScriptGuard(JSContext *cx)
+      : cx_(cx), script_(cx)
+    {
+        lookup_.str = NULL;
     }
 
     ~EvalScriptGuard() {
         if (script_) {
             CallDestroyScriptHook(cx_->runtime->defaultFreeOp(), script_);
             script_->isActiveEval = false;
             script_->isCachedEval = true;
-            script_->evalHashLink = *bucket_;
-            *bucket_ = script_;
+            if (lookup_.str && IsEvalCacheCandidate(script_))
+                cx_->runtime->evalCache.relookupOrAdd(p_, lookup_, script_);
         }
     }
 
-    void lookupInEvalCache(StackFrame *caller, unsigned staticLevel,
-                           JSPrincipals *principals, JSObject &scopeobj) {
-        if (JSScript *found = EvalCacheLookup(cx_, str_, caller, staticLevel,
-                                              principals, scopeobj, bucket_)) {
-            js_CallNewScriptHook(cx_, found, NULL);
-            script_ = found;
+    void lookupInEvalCache(JSLinearString *str, JSFunction *caller, unsigned staticLevel)
+    {
+        lookup_.str = str;
+        lookup_.caller = caller;
+        lookup_.staticLevel = staticLevel;
+        lookup_.version = cx_->findVersion();
+        lookup_.compartment = cx_->compartment;
+        p_ = cx_->runtime->evalCache.lookupForAdd(lookup_);
+        if (p_) {
+            script_ = *p_;
+            cx_->runtime->evalCache.remove(p_);
+            js_CallNewScriptHook(cx_, script_, NULL);
             script_->isCachedEval = false;
             script_->isActiveEval = true;
         }
     }
 
     void setNewScript(JSScript *script) {
         // JSScript::initFromEmitter has already called js_CallNewScriptHook.
         JS_ASSERT(!script_ && script);
@@ -242,21 +183,16 @@ EvalKernel(JSContext *cx, const CallArgs
         staticLevel = caller->script()->staticLevel + 1;
 
         // Direct calls to eval are supposed to see the caller's |this|. If we
         // haven't wrapped that yet, do so now, before we make a copy of it for
         // the eval code to use.
         if (!ComputeThis(cx, caller))
             return false;
         thisv = caller->thisValue();
-
-#ifdef DEBUG
-        jsbytecode *callerPC = caller->pcQuadratic(cx);
-        JS_ASSERT(callerPC && JSOp(*callerPC) == JSOP_EVAL);
-#endif
     } else {
         JS_ASSERT(args.callee().global() == *scopeobj);
         staticLevel = 0;
 
         // Use the global as 'this', modulo outerization.
         JSObject *thisobj = scopeobj->thisObject(cx);
         if (!thisobj)
             return false;
@@ -305,22 +241,22 @@ EvalKernel(JSContext *cx, const CallArgs
                 if (tmp.isUndefined())
                     break;
                 args.rval() = tmp;
                 return true;
             }
         }
     }
 
-    EvalScriptGuard esg(cx, linearStr);
+    EvalScriptGuard esg(cx);
 
     JSPrincipals *principals = PrincipalsForCompiledCode(args, cx);
 
     if (evalType == DIRECT_EVAL && caller->isNonEvalFunctionFrame())
-        esg.lookupInEvalCache(caller, staticLevel, principals, *scopeobj);
+        esg.lookupInEvalCache(linearStr, caller->fun(), staticLevel);
 
     if (!esg.foundScript()) {
         unsigned lineno;
         const char *filename;
         JSPrincipals *originPrincipals;
         CurrentScriptFileLineOrigin(cx, &filename, &lineno, &originPrincipals,
                                     evalType == DIRECT_EVAL ? CALLED_FROM_JSOP_EVAL
                                                             : NOT_CALLED_FROM_JSOP_EVAL);
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -871,16 +871,19 @@ JSRuntime::init(uint32_t maxbytes)
         return false;
 
     if (!stackSpace.init())
         return false;
 
     if (!scriptFilenameTable.init())
         return false;
 
+    if (!evalCache.init())
+        return false;
+
     debugScopes = this->new_<DebugScopes>(this);
     if (!debugScopes || !debugScopes->init()) {
         Foreground::delete_(debugScopes);
         return false;
     }
 
     nativeStackBase = GetNativeStackBase();
     return true;
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -168,28 +168,35 @@ class ToSourceCache
     Map *map_;
   public:
     ToSourceCache() : map_(NULL) {}
     JSString *lookup(JSFunction *fun);
     void put(JSFunction *fun, JSString *);
     void purge();
 };
 
-class EvalCache
+struct EvalCacheLookup
 {
-    static const unsigned SHIFT = 6;
-    static const unsigned LENGTH = 1 << SHIFT;
-    JSScript *table_[LENGTH];
+    JSLinearString *str;
+    JSFunction *caller;
+    unsigned staticLevel;
+    JSVersion version;
+    JSCompartment *compartment;
+};
 
-  public:
-    EvalCache() { PodArrayZero(table_); }
-    JSScript **bucket(JSLinearString *str);
-    void purge();
+struct EvalCacheHashPolicy
+{
+    typedef EvalCacheLookup Lookup;
+
+    static HashNumber hash(const Lookup &l);
+    static bool match(JSScript *script, const EvalCacheLookup &l);
 };
 
+typedef HashSet<JSScript *, EvalCacheHashPolicy, SystemAllocPolicy> EvalCache;
+
 class NativeIterCache
 {
     static const size_t SIZE = size_t(1) << 8;
 
     /* Cached native iterators. */
     JSObject            *data[SIZE];
 
     static size_t getIndex(uint32_t key) {
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -3051,17 +3051,17 @@ PurgeRuntime(JSTracer *trc)
 
     rt->tempLifoAlloc.freeUnused();
 
     rt->gsnCache.purge();
     rt->propertyCache.purge(rt);
     rt->newObjectCache.purge();
     rt->nativeIterCache.purge();
     rt->toSourceCache.purge();
-    rt->evalCache.purge();
+    rt->evalCache.clear();
 
     for (ContextIter acx(rt); !acx.done(); acx.next())
         acx->purge();
 }
 
 static bool
 ShouldPreserveJITCode(JSCompartment *c, int64_t currentTime)
 {
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -402,19 +402,16 @@ struct JSScript : public js::gc::Cell
                                    comment above Create() for details) */
 
     const char      *filename;  /* source filename or null */
     js::HeapPtrAtom *atoms;     /* maps immediate index to literal struct */
 
     JSPrincipals    *principals;/* principals for this script */
     JSPrincipals    *originPrincipals; /* see jsapi.h 'originPrincipals' comment */
 
-    /* The next link in the eval cache */
-    js::HeapPtrScript evalHashLink;
-
     /* Persistent type information retained across GCs. */
     js::types::TypeScript *types;
 
   private:
 #ifdef JS_METHODJIT
     JITScriptSet *jitInfo;
 #endif
 
@@ -432,16 +429,20 @@ struct JSScript : public js::gc::Cell
 
     uint32_t        natoms;     /* length of atoms array */
 
   private:
     uint32_t        useCount;   /* Number of times the script has been called
                                  * or has had backedges taken. Reset if the
                                  * script's JIT code is forcibly discarded. */
 
+#if JS_BITS_PER_WORD == 32
+    uint32_t        pad32;
+#endif
+
 #ifdef DEBUG
     // Unique identifier within the compartment for this script, used for
     // printing analysis information.
     uint32_t        id_;
   private:
     uint32_t        idpad;
 #endif