AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[esnext] fix tagged template caching strategy for eval caching

Browse Source 
Previously, eval caching was only disabled if the root eval body code
contained a tagged template. Per discussion on
https://github.com/tc39/ecma262/pull/890, this is incorrect.

This change tracks if eval caching is allowed during parsing, and
uses this information to decide to insert
new entries into the cache, or not.

This change also removes the TemplateObject feedback kind, as it's no
longer needed (behaves the same as Literal feedback).

BUG=v8:3230, v8:2891
R=littledan@chromium.org, yangguo@chromium.org, bmeurer@chromium.org,
rmcilroy@chromium.org

Change-Id: Ib75abe9159baf4d8ad10f8de99d2152714bd0094
Reviewed-on: https://chromium-review.googlesource.com/916945
Commit-Queue: Caitlin Potter <caitp@igalia.com>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51373}
This commit is contained in:
Caitlin Potter 2018-02-16 14:39:53 -05:00 committed by Commit Bot
parent 431c473b74
commit e56eac022f
14 changed files with 177 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/compilation-cache.cc
View File

@ -316,9 +316,6 @@ void CompilationCache::PutEval(Handle<String> source,
Handle<Cell> literals, int position) {
if (!IsEnabled()) return;
// TemplateObject feedback slots disable eval caching
if (function_info->feedback_metadata()->HasTemplateObjectSlot()) return;
HandleScope scope(isolate());
if (context->IsNativeContext()) {
eval_global_.Put(source, outer_info, function_info, context, literals,

25
src/compiler.cc
View File

@ -1088,9 +1088,11 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
Handle<SharedFunctionInfo> shared_info;
Handle<Script> script;
bool allow_eval_cache;
if (eval_result.has_shared()) {
shared_info = Handle<SharedFunctionInfo>(eval_result.shared(), isolate);
script = Handle<Script>(Script::cast(shared_info->script()), isolate);
allow_eval_cache = true;
} else {
script = isolate->factory()->NewScript(source);
if (isolate->NeedsSourcePositionsForProfiling()) {
@ -1134,6 +1136,7 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
if (!CompileToplevel(&parse_info, isolate).ToHandle(&shared_info)) {
return MaybeHandle<JSFunction>();
}
allow_eval_cache = parse_info.allow_eval_cache();
}
// If caller is strict mode, the result must be in strict mode as well.
@ -1148,20 +1151,24 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
result = isolate->factory()->NewFunctionFromSharedFunctionInfo(
shared_info, context, NOT_TENURED);
JSFunction::EnsureLiterals(result);
// Make sure to cache this result.
Handle<Cell> new_vector(result->feedback_vector_cell(), isolate);
compilation_cache->PutEval(source, outer_info, context, shared_info,
new_vector, eval_scope_position);
if (allow_eval_cache) {
// Make sure to cache this result.
Handle<Cell> new_vector(result->feedback_vector_cell(), isolate);
compilation_cache->PutEval(source, outer_info, context, shared_info,
new_vector, eval_scope_position);
}
}
} else {
result = isolate->factory()->NewFunctionFromSharedFunctionInfo(
shared_info, context, NOT_TENURED);
JSFunction::EnsureLiterals(result);
// Add the SharedFunctionInfo and the LiteralsArray to the eval cache if
// we didn't retrieve from there.
Handle<Cell> vector(result->feedback_vector_cell(), isolate);
compilation_cache->PutEval(source, outer_info, context, shared_info, vector,
eval_scope_position);
if (allow_eval_cache) {
// Add the SharedFunctionInfo and the LiteralsArray to the eval cache if
// we didn't retrieve from there.
Handle<Cell> vector(result->feedback_vector_cell(), isolate);
compilation_cache->PutEval(source, outer_info, context, shared_info,
vector, eval_scope_position);
}
}
// OnAfterCompile has to be called after we create the JSFunction, which we

12
src/feedback-vector-inl.h
View File

@ -35,16 +35,6 @@ int FeedbackMetadata::slot_count() const {
return Smi::ToInt(get(kSlotsCountIndex));
}
bool FeedbackMetadata::HasTemplateObjectSlot() const {
DisallowHeapAllocation no_gc;
FeedbackMetadataIterator iter(const_cast<FeedbackMetadata*>(this));
while (iter.HasNext()) {
iter.Next();
if (iter.kind() == FeedbackSlotKind::kTemplateObject) return true;
}
return false;
}
// static
FeedbackVector* FeedbackVector::cast(Object* obj) {
DCHECK(obj->IsFeedbackVector());
@ -58,7 +48,6 @@ int FeedbackMetadata::GetSlotSize(FeedbackSlotKind kind) {
case FeedbackSlotKind::kCompareOp:
case FeedbackSlotKind::kBinaryOp:
case FeedbackSlotKind::kLiteral:
case FeedbackSlotKind::kTemplateObject:
case FeedbackSlotKind::kCreateClosure:
case FeedbackSlotKind::kTypeProfile:
return 1;
@ -318,7 +307,6 @@ void FeedbackVector::ComputeCounts(int* with_type_info, int* generic,
}
case FeedbackSlotKind::kCreateClosure:
case FeedbackSlotKind::kLiteral:
case FeedbackSlotKind::kTemplateObject:
break;
case FeedbackSlotKind::kInvalid:
case FeedbackSlotKind::kKindsNumber:

5
src/feedback-vector.cc
View File

@ -173,8 +173,6 @@ const char* FeedbackMetadata::Kind2String(FeedbackSlotKind kind) {
return "kCreateClosure";
case FeedbackSlotKind::kLiteral:
return "Literal";
case FeedbackSlotKind::kTemplateObject:
return "TemplateObject";
case FeedbackSlotKind::kTypeProfile:
return "TypeProfile";
case FeedbackSlotKind::kForIn:
@ -257,7 +255,6 @@ Handle<FeedbackVector> FeedbackVector::New(Isolate* isolate,
break;
}
case FeedbackSlotKind::kLiteral:
case FeedbackSlotKind::kTemplateObject:
vector->set(index, Smi::kZero, SKIP_WRITE_BARRIER);
break;
case FeedbackSlotKind::kCall:
@ -464,7 +461,6 @@ bool FeedbackNexus::Clear() {
break;
case FeedbackSlotKind::kLiteral:
case FeedbackSlotKind::kTemplateObject:
SetFeedback(Smi::kZero, SKIP_WRITE_BARRIER);
feedback_updated = true;
break;
@ -535,7 +531,6 @@ InlineCacheState FeedbackNexus::StateFromFeedback() const {
switch (kind()) {
case FeedbackSlotKind::kCreateClosure:
case FeedbackSlotKind::kLiteral:
case FeedbackSlotKind::kTemplateObject:
// CreateClosure and literal slots don't have a notion of state.
UNREACHABLE();
break;

6
src/feedback-vector.h
View File

@ -48,7 +48,6 @@ enum class FeedbackSlotKind {
kTypeProfile,
kCreateClosure,
kLiteral,
kTemplateObject,
kForIn,
kInstanceOf,
@ -380,9 +379,6 @@ class V8_EXPORT_PRIVATE FeedbackVectorSpec {
}
FeedbackSlot AddLiteralSlot() { return AddSlot(FeedbackSlotKind::kLiteral); }
FeedbackSlot AddTemplateObjectSlot() {
return AddSlot(FeedbackSlotKind::kTemplateObject);
}
FeedbackSlot AddStoreDataPropertyInLiteralICSlot() {
return AddSlot(FeedbackSlotKind::kStoreDataPropertyInLiteral);
@ -431,8 +427,6 @@ class FeedbackMetadata : public FixedArray {
// Returns number of slots in the vector.
inline int slot_count() const;
inline bool HasTemplateObjectSlot() const;
// Returns slot kind for given slot.
FeedbackSlotKind GetKind(FeedbackSlot slot) const;

2
src/interpreter/bytecode-generator.cc
View File

@ -4174,7 +4174,7 @@ void BytecodeGenerator::VisitGetTemplateObject(GetTemplateObject* expr) {
builder()->SetExpressionPosition(expr);
size_t entry = builder()->AllocateDeferredConstantPoolEntry();
template_objects_.push_back(std::make_pair(expr, entry));
FeedbackSlot literal_slot = feedback_spec()->AddTemplateObjectSlot();
FeedbackSlot literal_slot = feedback_spec()->AddLiteralSlot();
builder()->GetTemplateObject(entry, feedback_index(literal_slot));
}

1
src/objects-printer.cc
View File

@ -842,7 +842,6 @@ void FeedbackNexus::Print(std::ostream& os) { // NOLINT
}
case FeedbackSlotKind::kCreateClosure:
case FeedbackSlotKind::kLiteral:
case FeedbackSlotKind::kTemplateObject:
case FeedbackSlotKind::kTypeProfile:
break;
case FeedbackSlotKind::kInvalid:

2
src/parsing/parse-info.h
View File

@ -85,6 +85,7 @@ class V8_EXPORT_PRIVATE ParseInfo {
set_on_background_thread)
FLAG_ACCESSOR(kWrappedAsFunction, is_wrapped_as_function,
set_wrapped_as_function)
FLAG_ACCESSOR(kAllowEvalCache, allow_eval_cache, set_allow_eval_cache)
#undef FLAG_ACCESSOR
void set_parse_restriction(ParseRestriction restriction) {
@ -263,6 +264,7 @@ class V8_EXPORT_PRIVATE ParseInfo {
kIsAsmWasmBroken = 1 << 12,
kOnBackgroundThread = 1 << 13,
kWrappedAsFunction = 1 << 14, // Implicitly wrapped as function.
kAllowEvalCache = 1 << 15,
};
//------------- Inputs to parsing and scope analysis -----------------------

11
src/parsing/parser-base.h
View File

@ -284,7 +284,8 @@ class ParserBase {
allow_harmony_dynamic_import_(false),
allow_harmony_import_meta_(false),
allow_harmony_optional_catch_binding_(false),
allow_harmony_private_fields_(false) {}
allow_harmony_private_fields_(false),
allow_eval_cache_(true) {}
#define ALLOW_ACCESSORS(name) \
bool allow_##name() const { return allow_##name##_; } \
@ -298,6 +299,7 @@ class ParserBase {
ALLOW_ACCESSORS(harmony_dynamic_import);
ALLOW_ACCESSORS(harmony_import_meta);
ALLOW_ACCESSORS(harmony_optional_catch_binding);
ALLOW_ACCESSORS(eval_cache);
#undef ALLOW_ACCESSORS
@ -1556,6 +1558,7 @@ class ParserBase {
bool allow_harmony_import_meta_;
bool allow_harmony_optional_catch_binding_;
bool allow_harmony_private_fields_;
bool allow_eval_cache_;
friend class DiscardableZoneScope;
};
@ -4672,6 +4675,12 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseTemplateLiteral(
// TEMPLATE_SPAN, or a TEMPLATE_TAIL.
DCHECK(peek() == Token::TEMPLATE_SPAN || peek() == Token::TEMPLATE_TAIL);
if (tagged) {
// TaggedTemplate expressions prevent the eval compilation cache from being
// used. This flag is only used if an eval is being parsed.
set_allow_eval_cache(false);
}
bool forbid_illegal_escapes = !tagged;
// If we reach a TEMPLATE_TAIL first, we are parsing a NoSubstitutionTemplate.

3
src/parsing/parser.cc
View File

@ -2913,6 +2913,9 @@ Parser::LazyParsingResult Parser::SkipFunction(
*ok = false;
return kLazyParsingComplete;
}
set_allow_eval_cache(reusable_preparser()->allow_eval_cache());
PreParserLogger* logger = reusable_preparser()->logger();
function_scope->set_end_position(logger->end());
Expect(Token::RBRACE, CHECK_OK_VALUE(kLazyParsingComplete));

1
src/parsing/parser.h
View File

@ -307,6 +307,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
SET_ALLOW(harmony_bigint);
SET_ALLOW(harmony_optional_catch_binding);
SET_ALLOW(harmony_private_fields);
SET_ALLOW(eval_cache);
#undef SET_ALLOW
}
return reusable_preparser_;

6
src/parsing/parsing.cc
View File

@ -45,6 +45,9 @@ bool ParseProgram(ParseInfo* info, Isolate* isolate) {
} else {
result->scope()->AttachOuterScopeInfo(info, isolate);
info->set_language_mode(info->literal()->language_mode());
if (info->is_eval()) {
info->set_allow_eval_cache(parser.allow_eval_cache());
}
}
parser.UpdateStatistics(isolate, info->script());
return (result != nullptr);
@ -79,6 +82,9 @@ bool ParseFunction(ParseInfo* info, Handle<SharedFunctionInfo> shared_info,
info->ast_value_factory());
} else {
result->scope()->AttachOuterScopeInfo(info, isolate);
if (info->is_eval()) {
info->set_allow_eval_cache(parser.allow_eval_cache());
}
}
parser.UpdateStatistics(isolate, info->script());
return (result != nullptr);

125
test/mjsunit/es6/templates.js
View File

@ -740,3 +740,128 @@ var global = this;
assertThrows(() => Function("f`foo`--"), ReferenceError);
assertThrows(() => Function("f`foo` = 1"), ReferenceError);
})();
// Disable eval caching if a tagged template occurs in a nested function
var v = 0;
var templates = [];
function tag(callSite) { templates.push(callSite); }
for (v = 0; v < 6; v += 2) {
eval("(function() { for (var i = 0; i < 2; ++i) tag`Hello${v}world` })()");
assertSame(templates[v], templates[v + 1]);
}
assertNotSame(templates[0], templates[2]);
assertNotSame(templates[0], templates[4]);
assertNotSame(templates[1], templates[3]);
assertNotSame(templates[1], templates[5]);
assertNotSame(templates[2], templates[4]);
assertNotSame(templates[3], templates[5]);
function makeSource1(id) {
return `function f() {
for (var i = 0; i < 2; ++i) tag\`Hello${id}world\`;
}
f();`;
}
templates = [];
for (v = 0; v < 6; v += 2) {
eval(makeSource1(v));
assertSame(templates[v], templates[v + 1]);
}
assertNotSame(templates[0], templates[2]);
assertNotSame(templates[0], templates[4]);
assertNotSame(templates[1], templates[3]);
assertNotSame(templates[1], templates[5]);
assertNotSame(templates[2], templates[4]);
assertNotSame(templates[3], templates[5]);
templates = [];
eval("(function() { for (var i = 0; i < 2; ++i) tag`Hello${1}world` })()");
eval("(function() { for (var i = 0; i < 2; ++i) tag`Hello${2}world` })()");
eval("(function() { for (var i = 0; i < 2; ++i) tag`Hello${2}world` })()");
assertSame(templates[0], templates[1]);
assertNotSame(templates[0], templates[2]);
assertNotSame(templates[0], templates[4]);
assertNotSame(templates[1], templates[3]);
assertNotSame(templates[1], templates[5]);
assertSame(templates[2], templates[3]);
assertNotSame(templates[2], templates[4]);
assertNotSame(templates[3], templates[5]);
assertSame(templates[4],templates[5]);
templates = [];
eval(makeSource1(1));
eval(makeSource1(2));
eval(makeSource1(3));
assertSame(templates[0], templates[1]);
assertNotSame(templates[0], templates[2]);
assertNotSame(templates[0], templates[4]);
assertNotSame(templates[1], templates[3]);
assertNotSame(templates[1], templates[5]);
assertSame(templates[2], templates[3]);
assertNotSame(templates[2], templates[4]);
assertNotSame(templates[3], templates[5]);
assertSame(templates[4],templates[5]);
// Disable eval caching if a tagged template occurs in an even deeper nested function
var v = 0;
templates = [];
for (v = 0; v < 6; v += 2) {
eval("(function() { (function() { for (var i = 0; i < 2; ++i) tag`Hello${v}world` })() })()");
if (!v) continue;
assertNotSame(templates[v], templates[v - 1]);
}
assertNotSame(templates[0], templates[2]);
assertNotSame(templates[0], templates[4]);
assertNotSame(templates[1], templates[3]);
assertNotSame(templates[1], templates[5]);
assertNotSame(templates[2], templates[4]);
assertNotSame(templates[3], templates[5]);
function makeSource2(id) {
return `function f() {
function innerF() {
for (var i = 0; i < 2; ++i) tag\`Hello${id}world\`;
}
return innerF();
}
f();`;
}
templates = [];
for (v = 0; v < 6; v += 2) {
eval(makeSource2(v));
assertSame(templates[v], templates[v + 1]);
}
assertNotSame(templates[0], templates[2]);
assertNotSame(templates[0], templates[4]);
assertNotSame(templates[1], templates[3]);
assertNotSame(templates[1], templates[5]);
assertNotSame(templates[2], templates[4]);
assertNotSame(templates[3], templates[5]);
templates = [];
eval("(function() { (function() { for (var i = 0; i < 2; ++i) tag`Hello${1}world` })() })()");
eval("(function() { (function() { for (var i = 0; i < 2; ++i) tag`Hello${2}world` })() })()");
eval("(function() { (function() { for (var i = 0; i < 2; ++i) tag`Hello${2}world` })() })()");
assertSame(templates[0], templates[1]);
assertNotSame(templates[0], templates[2]);
assertNotSame(templates[0], templates[4]);
assertNotSame(templates[1], templates[3]);
assertNotSame(templates[1], templates[5]);
assertSame(templates[2], templates[3]);
assertNotSame(templates[2], templates[4]);
assertNotSame(templates[3], templates[5]);
assertSame(templates[4], templates[5]);
templates = [];
eval(makeSource2(1));
eval(makeSource2(2));
eval(makeSource2(3));
assertSame(templates[0], templates[1]);
assertNotSame(templates[0], templates[2]);
assertNotSame(templates[0], templates[4]);
assertNotSame(templates[1], templates[3]);
assertNotSame(templates[1], templates[5]);
assertSame(templates[2], templates[3]);
assertNotSame(templates[2], templates[4]);
assertNotSame(templates[3], templates[5]);
assertSame(templates[4], templates[5]);

13
test/mjsunit/mjsunit.js
View File

@ -55,6 +55,9 @@ MjsUnitAssertionError.prototype.toString = function () {
// For known primitive values, please use assertEquals.
var assertSame;
// Inverse of assertSame.
var assertNotSame;
// Expected and found values are identical primitive values or functions
// or similarly structured objects (checking internal properties
// of, e.g., Number and Date objects, the elements of arrays
@ -375,6 +378,16 @@ var failWithMessage;
fail(PrettyPrint(expected), found, name_opt);
};
assertNotSame = function assertNotSame(expected, found, name_opt) {
// TODO(mstarzinger): We should think about using Harmony's egal operator
// or the function equivalent Object.is() here.
if (found !== expected) {
if (expected === 0 || (1 / expected) !== (1 / found)) return;
} else if (!((expected !== expected) && (found !== found))) {
return;
}
fail(PrettyPrint(expected), found, name_opt);
}
assertEquals = function assertEquals(expected, found, name_opt) {
if (!deepEquals(found, expected)) {