Bug 1260894 - Remove broken LazyScriptCache draft
authorTed Campbell <tcampbell@mozilla.com>
Thu, 09 Nov 2017 10:04:00 -0500
changeset 695628 5c7e156ac9eed70654b42d647768caecbe0d71ef
parent 688758 a89e5587c7a761fa59b82270b861c7f547968145
child 739657 e7e2f3579ee17d83d96264fd7429b9b8ecf21827
push id88485
push userbmo:tcampbell@mozilla.com
push dateThu, 09 Nov 2017 15:09:48 +0000
bugs1260894
milestone58.0a1
Bug 1260894 - Remove broken LazyScriptCache Due to a shadowed variable in FixedSizeHash, the LazyScriptCache has not been successfully adding items to the cache. This has been broken for a while now and the lookup key and other constraints are no longer valid. This patch removes the cache altogether until someone can re-qualify the correctness and utility. MozReview-Commit-ID: CJoB6OpNZen
js/src/ds/FixedSizeHash.h
js/src/jsfun.cpp
js/src/jsscript.cpp
js/src/vm/Caches.h
js/src/vm/Runtime.h
deleted file mode 100644
--- a/js/src/ds/FixedSizeHash.h
+++ /dev/null
@@ -1,128 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
- * vim: set ts=8 sts=4 et sw=4 tw=99:
- * This Source Code Form is subject to the terms of the Mozilla Public
- * 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/. */
-
-#ifndef jsfixedsizehash_h_
-#define jsfixedsizehash_h_
-
-#include "ds/LifoAlloc.h"
-
-namespace js {
-
-/*
- * Class representing a hash set with fixed capacity, with newer entries
- * evicting older entries. Each entry has several hashes and can be stored in
- * different buckets, with the choice of which to evict on insertion being
- * managed via LRU. For tables with a relatively small size, using different
- * hashes increases utilization and makes it less likely that entries will keep
- * evicting each other due to wanting to use the same bucket.
- *
- * T indicates the type of hash elements, HashPolicy must have the following
- * contents:
- *
- * Lookup - As for HashMap / HashSet.
- *
- * bool match(T, Lookup) - As for HashMap / HashSet.
- *
- * NumHashes - Number of different hashes generated for each entry.
- *
- * void hash(Lookup, HashNumber[NumHashes]) - Compute all hashes for an entry.
- *
- * void clear(T*) - Clear an entry, such that isCleared() holds afterwards.
- *
- * bool isCleared(T) - Test whether an entry has been cleared.
- */
-template <class T, class HashPolicy, size_t Capacity>
-class FixedSizeHashSet
-{
-    T entries[Capacity];
-    uint32_t lastOperations[Capacity];
-    uint32_t numOperations;
-
-    static const size_t NumHashes = HashPolicy::NumHashes;
-
-    static_assert(Capacity > 0, "an empty fixed-size hash set is meaningless");
-
-  public:
-    typedef typename HashPolicy::Lookup Lookup;
-
-    FixedSizeHashSet()
-      : entries(), lastOperations(), numOperations(0)
-    {
-        MOZ_ASSERT(HashPolicy::isCleared(entries[0]));
-    }
-
-    bool lookup(const Lookup& lookup, T* pentry)
-    {
-        size_t bucket;
-        if (lookupReference(lookup, &bucket)) {
-            *pentry = entries[bucket];
-            lastOperations[bucket] = numOperations++;
-            return true;
-        }
-        return false;
-    }
-
-    void insert(const Lookup& lookup, const T& entry)
-    {
-        size_t buckets[NumHashes];
-        getBuckets(lookup, buckets);
-
-        size_t min = buckets[0];
-        for (size_t i = 0; i < NumHashes; i++) {
-            const T& entry = entries[buckets[i]];
-            if (HashPolicy::isCleared(entry)) {
-                entries[buckets[i]] = entry;
-                lastOperations[buckets[i]] = numOperations++;
-                return;
-            }
-            if (i && lastOperations[min] > lastOperations[buckets[i]])
-                min = buckets[i];
-        }
-
-        entries[min] = entry;
-        lastOperations[min] = numOperations++;
-    }
-
-    template <typename S>
-    void remove(const S& s)
-    {
-        size_t bucket;
-        if (lookupReference(s, &bucket))
-            HashPolicy::clear(&entries[bucket]);
-    }
-
-  private:
-    template <typename S>
-    bool lookupReference(const S& s, size_t* pbucket)
-    {
-        size_t buckets[NumHashes];
-        getBuckets(s, buckets);
-
-        for (size_t i = 0; i < NumHashes; i++) {
-            const T& entry = entries[buckets[i]];
-            if (!HashPolicy::isCleared(entry) && HashPolicy::match(entry, s)) {
-                *pbucket = buckets[i];
-                return true;
-            }
-        }
-
-        return false;
-    }
-
-    template <typename S>
-    void getBuckets(const S& s, size_t buckets[NumHashes])
-    {
-        HashNumber hashes[NumHashes];
-        HashPolicy::hash(s, hashes);
-
-        for (size_t i = 0; i < NumHashes; i++)
-            buckets[i] = hashes[i] % Capacity;
-    }
-};
-
-}  /* namespace js */
-
-#endif /* jsfixedsizehash_h_ */
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -1586,44 +1586,16 @@ JSFunction::createScriptForLazilyInterpr
             script = lazy->functionNonDelazifying()->nonLazyScript();
             if (!script)
                 return false;
 
             fun->setUnlazifiedScript(script);
             return true;
         }
 
-        // Lazy script caching is only supported for leaf functions. If a
-        // script with inner functions was returned by the cache, those inner
-        // functions would be delazified when deep cloning the script, even if
-        // they have never executed.
-        //
-        // Additionally, the lazy script cache is not used during incremental
-        // GCs, to avoid resurrecting dead scripts after incremental sweeping
-        // has started.
-        if (canRelazify && !JS::IsIncrementalGCInProgress(cx)) {
-            LazyScriptCache::Lookup lookup(cx, lazy);
-            cx->caches().lazyScriptCache.lookup(lookup, script.address());
-        }
-
-        if (script) {
-            RootedScope enclosingScope(cx, lazy->enclosingScope());
-            RootedScript clonedScript(cx, CloneScriptIntoFunction(cx, enclosingScope, fun, script));
-            if (!clonedScript)
-                return false;
-
-            clonedScript->setSourceObject(lazy->sourceObject());
-
-            fun->initAtom(script->functionNonDelazifying()->displayAtom());
-
-            if (!lazy->maybeScript())
-                lazy->initScript(clonedScript);
-            return true;
-        }
-
         MOZ_ASSERT(lazy->scriptSource()->hasSourceData());
 
         // Parse and compile the script from source.
         size_t lazyLength = lazy->end() - lazy->begin();
         UncompressedSourceCache::AutoHoldEntry holder;
         ScriptSource::PinnedChars chars(cx, lazy->scriptSource(), holder,
                                         lazy->begin(), lazyLength);
         if (!chars.get())
@@ -1648,19 +1620,16 @@ JSFunction::createScriptForLazilyInterpr
 
         // Try to insert the newly compiled script into the lazy script cache.
         if (canRelazify) {
             // A script's starting column isn't set by the bytecode emitter, so
             // specify this from the lazy script so that if an identical lazy
             // script is encountered later a match can be determined.
             script->setColumn(lazy->column());
 
-            LazyScriptCache::Lookup lookup(cx, lazy);
-            cx->caches().lazyScriptCache.insert(lookup, script);
-
             // Remember the lazy script on the compiled script, so it can be
             // stored on the function again in case of re-lazification.
             // Only functions without inner functions are re-lazified.
             script->setLazyScript(lazy);
         }
 
         // XDR the newly delazified function.
         if (script->scriptSource()->hasEncoder()) {
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -3208,18 +3208,16 @@ JSScript::finalize(FreeOp* fop)
     if (data) {
         JS_POISON(data, 0xdb, computedSizeOfData());
         fop->free_(data);
     }
 
     if (scriptData_)
         scriptData_->decRefCount();
 
-    fop->runtime()->caches().lazyScriptCache.remove(this);
-
     // In most cases, our LazyScript's script pointer will reference this
     // script, and thus be nulled out by normal weakref processing. However, if
     // we unlazified the LazyScript during incremental sweeping, it will have a
     // completely different JSScript.
     MOZ_ASSERT_IF(lazyScript && !IsAboutToBeFinalizedUnbarriered(&lazyScript),
                   !lazyScript->hasScript() || lazyScript->maybeScriptUnbarriered() != this);
 }
 
@@ -4533,86 +4531,16 @@ JSScript::hasLoops()
 }
 
 bool
 JSScript::mayReadFrameArgsDirectly()
 {
     return argumentsHasVarBinding() || hasRest();
 }
 
-static inline void
-LazyScriptHash(uint32_t lineno, uint32_t column, uint32_t begin, uint32_t end,
-               HashNumber hashes[3])
-{
-    HashNumber hash = lineno;
-    hash = RotateLeft(hash, 4) ^ column;
-    hash = RotateLeft(hash, 4) ^ begin;
-    hash = RotateLeft(hash, 4) ^ end;
-
-    hashes[0] = hash;
-    hashes[1] = RotateLeft(hashes[0], 4) ^ begin;
-    hashes[2] = RotateLeft(hashes[1], 4) ^ end;
-}
-
-void
-LazyScriptHashPolicy::hash(const Lookup& lookup, HashNumber hashes[3])
-{
-    LazyScript* lazy = lookup.lazy;
-    LazyScriptHash(lazy->lineno(), lazy->column(), lazy->begin(), lazy->end(), hashes);
-}
-
-void
-LazyScriptHashPolicy::hash(JSScript* script, HashNumber hashes[3])
-{
-    LazyScriptHash(script->lineno(), script->column(), script->sourceStart(), script->sourceEnd(), hashes);
-}
-
-bool
-LazyScriptHashPolicy::match(JSScript* script, const Lookup& lookup)
-{
-    JSContext* cx = lookup.cx;
-    LazyScript* lazy = lookup.lazy;
-
-    // To be a match, the script and lazy script need to have the same line
-    // and column and to be at the same position within their respective
-    // source blobs, and to have the same source contents and version.
-    //
-    // While the surrounding code in the source may differ, this is
-    // sufficient to ensure that compiling the lazy script will yield an
-    // identical result to compiling the original script.
-    //
-    // Note that the filenames and origin principals of the lazy script and
-    // original script can differ. If there is a match, these will be fixed
-    // up in the resulting clone by the caller.
-
-    if (script->lineno() != lazy->lineno() ||
-        script->column() != lazy->column() ||
-        script->getVersion() != lazy->version() ||
-        script->sourceStart() != lazy->begin() ||
-        script->sourceEnd() != lazy->end())
-    {
-        return false;
-    }
-
-    UncompressedSourceCache::AutoHoldEntry holder;
-
-    size_t scriptBegin = script->sourceStart();
-    size_t length = script->sourceEnd() - scriptBegin;
-    ScriptSource::PinnedChars scriptChars(cx, script->scriptSource(), holder, scriptBegin, length);
-    if (!scriptChars.get())
-        return false;
-
-    MOZ_ASSERT(scriptBegin == lazy->begin());
-    ScriptSource::PinnedChars lazyChars(cx, lazy->scriptSource(), holder, scriptBegin, length);
-    if (!lazyChars.get())
-        return false;
-
-    return !memcmp(scriptChars.get(), lazyChars.get(), length);
-}
-
 void
 JSScript::AutoDelazify::holdScript(JS::HandleFunction fun)
 {
     if (fun) {
         if (fun->compartment()->isSelfHosting) {
             // The self-hosting compartment is shared across runtimes, so we
             // can't use JSAutoCompartment: it could cause races. Functions in
             // the self-hosting compartment will never be lazy, so we can safely
--- a/js/src/vm/Caches.h
+++ b/js/src/vm/Caches.h
@@ -8,17 +8,16 @@
 #define vm_Caches_h
 
 #include "jsatom.h"
 #include "jsbytecode.h"
 #include "jsmath.h"
 #include "jsobj.h"
 #include "jsscript.h"
 
-#include "ds/FixedSizeHash.h"
 #include "frontend/SourceNotes.h"
 #include "gc/Tracer.h"
 #include "js/RootingAPI.h"
 #include "js/UniquePtr.h"
 #include "vm/NativeObject.h"
 
 namespace js {
 
@@ -81,43 +80,16 @@ struct EvalCacheHashPolicy
     typedef EvalCacheLookup Lookup;
 
     static HashNumber hash(const Lookup& l);
     static bool match(const EvalCacheEntry& entry, const EvalCacheLookup& l);
 };
 
 typedef HashSet<EvalCacheEntry, EvalCacheHashPolicy, SystemAllocPolicy> EvalCache;
 
-struct LazyScriptHashPolicy
-{
-    struct Lookup {
-        JSContext* cx;
-        LazyScript* lazy;
-
-        Lookup(JSContext* cx, LazyScript* lazy)
-          : cx(cx), lazy(lazy)
-        {}
-    };
-
-    static const size_t NumHashes = 3;
-
-    static void hash(const Lookup& lookup, HashNumber hashes[NumHashes]);
-    static bool match(JSScript* script, const Lookup& lookup);
-
-    // Alternate methods for use when removing scripts from the hash without an
-    // explicit LazyScript lookup.
-    static void hash(JSScript* script, HashNumber hashes[NumHashes]);
-    static bool match(JSScript* script, JSScript* lookup) { return script == lookup; }
-
-    static void clear(JSScript** pscript) { *pscript = nullptr; }
-    static bool isCleared(JSScript* script) { return !script; }
-};
-
-typedef FixedSizeHashSet<JSScript*, LazyScriptHashPolicy, 769> LazyScriptCache;
-
 /*
  * Cache for speeding up repetitive creation of objects in the VM.
  * When an object is created which matches the criteria in the 'key' section
  * below, an entry is filled with the resulting object.
  */
 class NewObjectCache
 {
     /* Statically asserted to be equal to sizeof(JSObject_Slots16) */
@@ -251,17 +223,16 @@ class RuntimeCaches
     js::MathCache* createMathCache(JSContext* cx);
 
   public:
     js::GSNCache gsnCache;
     js::EnvironmentCoordinateNameCache envCoordinateNameCache;
     js::NewObjectCache newObjectCache;
     js::UncompressedSourceCache uncompressedSourceCache;
     js::EvalCache evalCache;
-    LazyScriptCache lazyScriptCache;
 
     bool init();
 
     js::MathCache* getMathCache(JSContext* cx) {
         return mathCache_ ? mathCache_.get() : createMathCache(cx);
     }
     js::MathCache* maybeGetMathCache() {
         return mathCache_.get();
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -23,17 +23,16 @@
 #include "jsscript.h"
 
 #ifdef XP_DARWIN
 # include "wasm/WasmSignalHandlers.h"
 #endif
 #include "builtin/AtomicsObject.h"
 #include "builtin/Intl.h"
 #include "builtin/Promise.h"
-#include "ds/FixedSizeHash.h"
 #include "frontend/NameCollections.h"
 #include "gc/GCRuntime.h"
 #include "gc/Tracer.h"
 #include "gc/ZoneGroup.h"
 #include "irregexp/RegExpStack.h"
 #include "js/Debug.h"
 #include "js/GCVector.h"
 #include "js/HashTable.h"