Skip to content

Commit

Permalink
Bug 1260894 - Remove broken LazyScriptCache r=jorendorff
Browse files Browse the repository at this point in the history
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
  • Loading branch information
moztcampbell committed Nov 9, 2017
1 parent 8419c7c commit 7dd6fa0
Show file tree
Hide file tree
Showing 5 changed files with 0 additions and 261 deletions.
128 changes: 0 additions & 128 deletions js/src/ds/FixedSizeHash.h

This file was deleted.

31 changes: 0 additions & 31 deletions js/src/jsfun.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1592,34 +1592,6 @@ JSFunction::createScriptForLazilyInterpretedFunction(JSContext* cx, HandleFuncti
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.
Expand Down Expand Up @@ -1654,9 +1626,6 @@ JSFunction::createScriptForLazilyInterpretedFunction(JSContext* cx, HandleFuncti
// 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.
Expand Down
72 changes: 0 additions & 72 deletions js/src/jsscript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3205,8 +3205,6 @@ JSScript::finalize(FreeOp* fop)
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
Expand Down Expand Up @@ -4529,76 +4527,6 @@ 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)
{
Expand Down
29 changes: 0 additions & 29 deletions js/src/vm/Caches.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "jsobj.h"
#include "jsscript.h"

#include "ds/FixedSizeHash.h"
#include "frontend/SourceNotes.h"
#include "gc/Tracer.h"
#include "js/RootingAPI.h"
Expand Down Expand Up @@ -86,33 +85,6 @@ struct EvalCacheHashPolicy

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
Expand Down Expand Up @@ -256,7 +228,6 @@ class RuntimeCaches
js::NewObjectCache newObjectCache;
js::UncompressedSourceCache uncompressedSourceCache;
js::EvalCache evalCache;
LazyScriptCache lazyScriptCache;

bool init();

Expand Down
1 change: 0 additions & 1 deletion js/src/vm/Runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#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"
Expand Down

0 comments on commit 7dd6fa0

Please sign in to comment.