[Runtime] Ensure template objects are retained if bytecode is flushed.

Template objects should be cached after they are first created and reused on
subsiquent calls to tag functions. Currently these cached objects are stored
on the feedback vector, which has appropriate lifetime, however with bytecode
flushing the feedback vector could be cleared when the bytecode is flushed,
causing the template object to be dropped.

In order to retain the cached template objects in the face of bytecode flushing,
this CL adds a weakmap for each native context that is (weakly) keyed by
shared function info, and holds a linked list of cached template objects
associated with that shared function info, indexed by feedback vector slot id.
Misses will check this weakmap, and if no entry is found, a new template object
is created and added into this weakmap alongside the feedback vector.

BUG=v8:8799,v8:8799,v8:8395

Change-Id: Ia95d5cfc394ce58dc9fe6a1e49780f05299acc17
Reviewed-on: https://chromium-review.googlesource.com/c/1477746
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59818}

BUILD.gn

@@ -2392,6 +2392,7 @@ v8_source_set("v8_base") {
     "src/objects/string.h",
     "src/objects/struct-inl.h",
     "src/objects/struct.h",
+    "src/objects/template-objects-inl.h",
     "src/objects/template-objects.cc",
     "src/objects/template-objects.h",
     "src/objects/templates-inl.h",

src/code-stub-assembler.cc

@@ -13795,11 +13795,4 @@ TNode<String> CodeStubAssembler::TaggedToDirectString(TNode<Object> value,
 }
 
 }  // namespace internal
-
-// TODO(pwong): Remove. This is a workaround for crbug.com/v8/8719
-namespace {}  // namespace
-
-// TODO(pwong): Remove. This is a workaround for crbug.com/v8/8719
-namespace {}  // namespace
-
 }  // namespace v8

src/compiler/bytecode-graph-builder.cc

@@ -18,6 +18,7 @@
 #include "src/objects/js-generator.h"
 #include "src/objects/literal-objects-inl.h"
 #include "src/objects/smi.h"
+#include "src/objects/template-objects-inl.h"
 #include "src/vector-slot-pair.h"
 
 namespace v8 {
@@ -550,6 +551,7 @@ BytecodeGraphBuilder::BytecodeGraphBuilder(
       state_values_cache_(jsgraph),
       source_positions_(source_positions),
       start_position_(shared_info->StartPosition(), inlining_id),
+      shared_info_(shared_info),
       native_context_(native_context) {}
 
 Node* BytecodeGraphBuilder::GetFunctionClosure() {
@@ -1696,8 +1698,8 @@ void BytecodeGraphBuilder::VisitGetTemplateObject() {
     // It's not observable when the template object is created, so we
     // can just create it eagerly during graph building and bake in
     // the JSArray constant here.
-    cached_value =
-        TemplateObjectDescription::CreateTemplateObject(isolate(), description);
+    cached_value = TemplateObjectDescription::GetTemplateObject(
+        isolate(), native_context(), description, shared_info(), slot.ToInt());
     nexus.vector()->Set(slot, *cached_value);
   } else {
     cached_value =

src/compiler/bytecode-graph-builder.h

@@ -371,6 +371,8 @@ class BytecodeGraphBuilder {
     needs_eager_checkpoint_ = value;
   }
 
+  Handle<SharedFunctionInfo> shared_info() const { return shared_info_; }
+
   Handle<Context> native_context() const { return native_context_; }
 
 #define DECLARE_VISIT_BYTECODE(name, ...) void Visit##name();
@@ -432,6 +434,8 @@ class BytecodeGraphBuilder {
 
   SourcePosition const start_position_;
 
+  Handle<SharedFunctionInfo> const shared_info_;
+
   // The native context for which we optimize.
   Handle<Context> const native_context_;
 

src/contexts.h

@@ -302,6 +302,7 @@ enum ContextLookupFlags {
   V(WASM_MEMORY_CONSTRUCTOR_INDEX, JSFunction, wasm_memory_constructor)        \
   V(WASM_MODULE_CONSTRUCTOR_INDEX, JSFunction, wasm_module_constructor)        \
   V(WASM_TABLE_CONSTRUCTOR_INDEX, JSFunction, wasm_table_constructor)          \
+  V(TEMPLATE_WEAKMAP_INDEX, HeapObject, template_weakmap)                      \
   V(TYPED_ARRAY_FUN_INDEX, JSFunction, typed_array_function)                   \
   V(TYPED_ARRAY_PROTOTYPE_INDEX, JSObject, typed_array_prototype)              \
   V(UINT16_ARRAY_FUN_INDEX, JSFunction, uint16_array_fun)                      \

src/heap/factory.cc

@@ -46,6 +46,7 @@
 #include "src/objects/scope-info.h"
 #include "src/objects/stack-frame-info-inl.h"
 #include "src/objects/struct-inl.h"
+#include "src/objects/template-objects-inl.h"
 #include "src/transitions-inl.h"
 #include "src/unicode-cache.h"
 #include "src/unicode-inl.h"

src/interpreter/bytecode-generator.cc

@@ -18,6 +18,7 @@
 #include "src/objects/debug-objects.h"
 #include "src/objects/literal-objects-inl.h"
 #include "src/objects/smi.h"
+#include "src/objects/template-objects-inl.h"
 #include "src/parsing/parse-info.h"
 #include "src/parsing/token.h"
 #include "src/unoptimized-compilation-info.h"

src/interpreter/interpreter-generator.cc

@@ -2596,9 +2596,13 @@ IGNITION_HANDLER(GetTemplateObject, InterpreterAssembler) {
   BIND(&call_runtime);
   {
     Node* description = LoadConstantPoolEntryAtOperandIndex(0);
+    Node* slot_smi = SmiTag(slot);
+    Node* closure = LoadRegister(Register::function_closure());
+    Node* shared_info =
+        LoadObjectField(closure, JSFunction::kSharedFunctionInfoOffset);
     Node* context = GetContext();
-    Node* result =
-        CallRuntime(Runtime::kCreateTemplateObject, context, description);
+    Node* result = CallRuntime(Runtime::kGetTemplateObject, context,
+                               description, shared_info, slot_smi);
 
     Label end(this);
     GotoIf(IsUndefined(feedback_vector), &end);

src/objects-inl.h

@@ -32,9 +32,9 @@
 #include "src/objects/oddball.h"
 #include "src/objects/regexp-match-info.h"
 #include "src/objects/scope-info.h"
+#include "src/objects/shared-function-info.h"
 #include "src/objects/slots-inl.h"
 #include "src/objects/smi-inl.h"
-#include "src/objects/template-objects.h"
 #include "src/objects/templates.h"
 #include "src/property-details.h"
 #include "src/property.h"
@@ -398,15 +398,12 @@ OBJECT_CONSTRUCTORS_IMPL(BigIntBase, HeapObject)
 OBJECT_CONSTRUCTORS_IMPL(BigInt, BigIntBase)
 OBJECT_CONSTRUCTORS_IMPL(FreshlyAllocatedBigInt, BigIntBase)
 
-OBJECT_CONSTRUCTORS_IMPL(TemplateObjectDescription, Tuple2)
-
 // ------------------------------------
 // Cast operations
 
 CAST_ACCESSOR(BigInt)
 CAST_ACCESSOR(RegExpMatchInfo)
 CAST_ACCESSOR(ScopeInfo)
-CAST_ACCESSOR(TemplateObjectDescription)
 
 bool Object::HasValidElements() {
   // Dictionary is covered under FixedArray.
@@ -837,10 +834,6 @@ Address HeapObject::GetFieldAddress(int field_offset) const {
   return FIELD_ADDR(*this, field_offset);
 }
 
-ACCESSORS(TemplateObjectDescription, raw_strings, FixedArray, kRawStringsOffset)
-ACCESSORS(TemplateObjectDescription, cooked_strings, FixedArray,
-          kCookedStringsOffset)
-
 // static
 Maybe<bool> Object::GreaterThan(Isolate* isolate, Handle<Object> x,
                                 Handle<Object> y) {
@@ -969,6 +962,10 @@ Object Object::GetSimpleHash(Object object) {
     uint32_t hash = BigInt::cast(object)->Hash();
     return Smi::FromInt(hash & Smi::kMaxValue);
   }
+  if (object->IsSharedFunctionInfo()) {
+    uint32_t hash = SharedFunctionInfo::cast(object)->Hash();
+    return Smi::FromInt(hash & Smi::kMaxValue);
+  }
   DCHECK(object->IsJSReceiver());
   return object;
 }

src/objects.cc

@@ -4872,6 +4872,15 @@ Script Script::Iterator::Next() {
   return Script();
 }
 
+uint32_t SharedFunctionInfo::Hash() {
+  // Hash SharedFunctionInfo based on its start position and script id. Note: we
+  // don't use the function's literal id since getting that is slow for compiled
+  // funcitons.
+  int start_pos = StartPosition();
+  int script_id = script()->IsScript() ? Script::cast(script())->id() : 0;
+  return static_cast<uint32_t>(base::hash_combine(start_pos, script_id));
+}
+
 Code SharedFunctionInfo::GetCode() const {
   // ======
   // NOTE: This chain of checks MUST be kept in sync with the equivalent CSA

src/objects.h

@@ -258,6 +258,7 @@ class AccessorPair;
 class AccessCheckInfo;
 class AllocationSite;
 class ByteArray;
+class CachedTemplateObject;
 class Cell;
 class ConsString;
 class DependentCode;
@@ -335,6 +336,7 @@ class ZoneForwardList;
   V(BreakPointInfo)                            \
   V(ByteArray)                                 \
   V(BytecodeArray)                             \
+  V(CachedTemplateObject)                      \
   V(CallHandlerInfo)                           \
   V(Callable)                                  \
   V(Cell)                                      \

src/objects/instance-type.h

@@ -415,6 +415,7 @@ V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os,
   V(CallHandlerInfo, CALL_HANDLER_INFO_TYPE)                                 \
   V(Cell, CELL_TYPE)                                                         \
   V(Code, CODE_TYPE)                                                         \
+  V(CachedTemplateObject, TUPLE3_TYPE)                                       \
   V(CodeDataContainer, CODE_DATA_CONTAINER_TYPE)                             \
   V(CoverageInfo, FIXED_ARRAY_TYPE)                                          \
   V(DescriptorArray, DESCRIPTOR_ARRAY_TYPE)                                  \

src/objects/shared-function-info.h

@@ -587,6 +587,9 @@ class SharedFunctionInfo : public HeapObject {
   static void EnsureSourcePositionsAvailable(
       Isolate* isolate, Handle<SharedFunctionInfo> shared_info);
 
+  // Hash based on function literal id and script id.
+  uint32_t Hash();
+
   inline bool construct_as_builtin() const;
 
   // Determines and sets the ConstructAsBuiltinBit in |flags|, based on the

src/objects/template-objects-inl.h

@@ -0,0 +1,37 @@
+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_OBJECTS_TEMPLATE_OBJECTS_INL_H_
+#define V8_OBJECTS_TEMPLATE_OBJECTS_INL_H_
+
+#include "src/objects/template-objects.h"
+
+#include "src/objects/js-array-inl.h"
+
+// Has to be the last include (doesn't have include guards):
+#include "src/objects/object-macros.h"
+
+namespace v8 {
+namespace internal {
+
+OBJECT_CONSTRUCTORS_IMPL(TemplateObjectDescription, Tuple2)
+OBJECT_CONSTRUCTORS_IMPL(CachedTemplateObject, Tuple3)
+
+CAST_ACCESSOR(TemplateObjectDescription)
+CAST_ACCESSOR(CachedTemplateObject)
+
+ACCESSORS(TemplateObjectDescription, raw_strings, FixedArray, kRawStringsOffset)
+ACCESSORS(TemplateObjectDescription, cooked_strings, FixedArray,
+          kCookedStringsOffset)
+
+SMI_ACCESSORS(CachedTemplateObject, slot_id, kSlotIdOffset)
+ACCESSORS(CachedTemplateObject, template_object, JSArray, kTemplateObjectOffset)
+ACCESSORS(CachedTemplateObject, next, HeapObject, kNextOffset)
+
+}  // namespace internal
+}  // namespace v8
+
+#include "src/objects/object-macros-undef.h"
+
+#endif  // V8_OBJECTS_TEMPLATE_OBJECTS_INL_H_

src/objects/template-objects.cc

@@ -4,17 +4,41 @@
 
 #include "src/objects/template-objects.h"
 
+#include "src/base/functional.h"
 #include "src/heap/factory.h"
 #include "src/isolate.h"
 #include "src/objects-inl.h"
+#include "src/objects/template-objects-inl.h"
 #include "src/property-descriptor.h"
 
 namespace v8 {
 namespace internal {
 
 // static
-Handle<JSArray> TemplateObjectDescription::CreateTemplateObject(
-    Isolate* isolate, Handle<TemplateObjectDescription> description) {
+Handle<JSArray> TemplateObjectDescription::GetTemplateObject(
+    Isolate* isolate, Handle<Context> native_context,
+    Handle<TemplateObjectDescription> description,
+    Handle<SharedFunctionInfo> shared_info, int slot_id) {
+  DCHECK(native_context->IsNativeContext());
+
+  // Check the template weakmap to see if the template object already exists.
+  Handle<EphemeronHashTable> template_weakmap =
+      native_context->template_weakmap()->IsUndefined(isolate)
+          ? EphemeronHashTable::New(isolate, 0)
+          : handle(EphemeronHashTable::cast(native_context->template_weakmap()),
+                   isolate);
+
+  uint32_t hash = shared_info->Hash();
+  Object maybe_cached_template = template_weakmap->Lookup(shared_info, hash);
+  while (!maybe_cached_template->IsTheHole()) {
+    CachedTemplateObject cached_template =
+        CachedTemplateObject::cast(maybe_cached_template);
+    if (cached_template->slot_id() == slot_id)
+      return handle(cached_template->template_object(), isolate);
+
+    maybe_cached_template = cached_template->next();
+  }
+
   // Create the raw object from the {raw_strings}.
   Handle<FixedArray> raw_strings(description->raw_strings(), isolate);
   Handle<JSArray> raw_object = isolate->factory()->NewJSArrayWithElements(
@@ -43,8 +67,30 @@ Handle<JSArray> TemplateObjectDescription::CreateTemplateObject(
   JSObject::SetIntegrityLevel(template_object, FROZEN, kThrowOnError)
       .ToChecked();
 
+  // Insert the template object into the template weakmap.
+  Handle<HeapObject> previous_cached_templates = handle(
+      HeapObject::cast(template_weakmap->Lookup(shared_info, hash)), isolate);
+  Handle<CachedTemplateObject> cached_template = CachedTemplateObject::New(
+      isolate, slot_id, template_object, previous_cached_templates);
+  template_weakmap = EphemeronHashTable::Put(
+      isolate, template_weakmap, shared_info, cached_template, hash);
+  native_context->set_template_weakmap(*template_weakmap);
+
   return template_object;
 }
 
+Handle<CachedTemplateObject> CachedTemplateObject::New(
+    Isolate* isolate, int slot_id, Handle<JSArray> template_object,
+    Handle<HeapObject> next) {
+  DCHECK(next->IsCachedTemplateObject() || next->IsTheHole());
+  Factory* factory = isolate->factory();
+  Handle<CachedTemplateObject> result = Handle<CachedTemplateObject>::cast(
+      factory->NewStruct(TUPLE3_TYPE, TENURED));
+  result->set_slot_id(slot_id);
+  result->set_template_object(*template_object);
+  result->set_next(*next);
+  return result;
+}
+
 }  // namespace internal
 }  // namespace v8

src/objects/template-objects.h

@@ -14,17 +14,40 @@
 namespace v8 {
 namespace internal {
 
-// TemplateObjectDescription is a triple of hash, raw strings and cooked
-// strings for tagged template literals. Used to communicate with the runtime
-// for template object creation within the {Runtime_CreateTemplateObject}
-// method.
+// CachedTemplateObject is a tuple used to cache a TemplateObject that has been
+// created. All the CachedTemplateObject's for a given SharedFunctionInfo form a
+// linked list via the next fields.
+class CachedTemplateObject final : public Tuple3 {
+ public:
+  DECL_INT_ACCESSORS(slot_id)
+  DECL_ACCESSORS(template_object, JSArray)
+  DECL_ACCESSORS(next, HeapObject)
+
+  static Handle<CachedTemplateObject> New(Isolate* isolate, int slot_id,
+                                          Handle<JSArray> template_object,
+                                          Handle<HeapObject> next);
+
+  DECL_CAST(CachedTemplateObject)
+
+  static constexpr int kSlotIdOffset = kValue1Offset;
+  static constexpr int kTemplateObjectOffset = kValue2Offset;
+  static constexpr int kNextOffset = kValue3Offset;
+
+  OBJECT_CONSTRUCTORS(CachedTemplateObject, Tuple3);
+};
+
+// TemplateObjectDescription is a tuple of raw strings and cooked strings for
+// tagged template literals. Used to communicate with the runtime for template
+// object creation within the {Runtime_GetTemplateObject} method.
 class TemplateObjectDescription final : public Tuple2 {
  public:
   DECL_ACCESSORS(raw_strings, FixedArray)
   DECL_ACCESSORS(cooked_strings, FixedArray)
 
-  static Handle<JSArray> CreateTemplateObject(
-      Isolate* isolate, Handle<TemplateObjectDescription> description);
+  static Handle<JSArray> GetTemplateObject(
+      Isolate* isolate, Handle<Context> native_context,
+      Handle<TemplateObjectDescription> description,
+      Handle<SharedFunctionInfo> shared_info, int slot_id);
 
   DECL_CAST(TemplateObjectDescription)
 

src/runtime/runtime-internal.cc

@@ -17,6 +17,7 @@
 #include "src/isolate-inl.h"
 #include "src/message-template.h"
 #include "src/objects/js-array-inl.h"
+#include "src/objects/template-objects-inl.h"
 #include "src/ostreams.h"
 #include "src/parsing/parse-info.h"
 #include "src/parsing/parsing.h"
@@ -671,12 +672,16 @@ RUNTIME_FUNCTION(Runtime_CreateAsyncFromSyncIterator) {
       Handle<JSReceiver>::cast(sync_iterator), next);
 }
 
-RUNTIME_FUNCTION(Runtime_CreateTemplateObject) {
+RUNTIME_FUNCTION(Runtime_GetTemplateObject) {
   HandleScope scope(isolate);
-  DCHECK_EQ(1, args.length());
+  DCHECK_EQ(3, args.length());
   CONVERT_ARG_HANDLE_CHECKED(TemplateObjectDescription, description, 0);
+  CONVERT_ARG_HANDLE_CHECKED(SharedFunctionInfo, shared_info, 1);
+  CONVERT_SMI_ARG_CHECKED(slot_id, 2);
 
-  return *TemplateObjectDescription::CreateTemplateObject(isolate, description);
+  Handle<Context> native_context(isolate->context()->native_context(), isolate);
+  return *TemplateObjectDescription::GetTemplateObject(
+      isolate, native_context, description, shared_info, slot_id);
 }
 
 RUNTIME_FUNCTION(Runtime_ReportMessage) {

src/runtime/runtime.h

@@ -214,10 +214,10 @@ namespace internal {
   F(CheckIsBootstrapping, 0, 1)                      \
   I(CreateAsyncFromSyncIterator, 1, 1)               \
   F(CreateListFromArrayLike, 1, 1)                   \
-  F(CreateTemplateObject, 1, 1)                      \
   F(FatalProcessOutOfMemoryInAllocateRaw, 0, 1)      \
   F(FatalProcessOutOfMemoryInvalidArrayLength, 0, 1) \
   F(GetAndResetRuntimeCallStats, -1 /* <= 2 */, 1)   \
+  F(GetTemplateObject, 3, 1)                         \
   F(IncrementUseCounter, 1, 1)                       \
   F(Interrupt, 0, 1)                                 \
   F(NewReferenceError, 2, 1)                         \

test/mjsunit/mjsunit.status

@@ -306,9 +306,6 @@
   # BUG(v8:4237)
   'regress/regress-3976': [SKIP],
 
-  # BUG(v8:8799): Bytecode flushing breaks tagged template literals
-  'es6/templates': [SKIP],
-
   # BUG(v8:8801): Bytecode flushing interacts poorly with assertOptimized
   'regress/regress-7254': [SKIP],
   'array-push5': [SKIP],

test/mjsunit/regress/regress-v8-8799.js

@@ -0,0 +1,11 @@
+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-gc --stress-flush-bytecode
+
+// Ensure tagged template objects are cached even after bytecode flushing.
+var f = (x) => eval`a${x}b`;
+var a = f();
+gc();
+assertSame(a, f());