From 5bf491cfeba9faaf20dbaef7b088173600d57dd4 Mon Sep 17 00:00:00 2001 From: Leszek Swirski Date: Wed, 28 Sep 2022 12:53:58 +0200 Subject: [PATCH] [runtime] Revert template cache work This reverts the following commits: * [runtime] Clean up dead entries in the template cache" 8436c0059cf2e6c29418c33fecd84dd696fb1aa6. * [runtime] Don't update template map for existing templates e7b9604040abc6f333a966b3988d721295eaab7e. * [runtime] Fix hash used in template cache caa087bb18bc87bb2a07239a01b1722f54b8382b. * [runtime] Hold cached template objects weakly 5d19e724d20448b5bfbaba7932123814ce933bad. * [runtime] Key template object cache on Script f3a0e8bccf6462c58ab7ea60ef83c7defcd98f9f. There are gerrit UI issues which appear to be template object caching related. For dashboard: This reverts commit 8436c0059cf2e6c29418c33fecd84dd696fb1aa6. This reverts commit e7b9604040abc6f333a966b3988d721295eaab7e. This reverts commit caa087bb18bc87bb2a07239a01b1722f54b8382b. This reverts commit 5d19e724d20448b5bfbaba7932123814ce933bad. This reverts commit f3a0e8bccf6462c58ab7ea60ef83c7defcd98f9f. Bug: v8:13190 Bug: chromium:1366900 Change-Id: I9759771441a4dece2a5dbb47e462ce0c0c01b182 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3925696 Commit-Queue: Leszek Swirski Auto-Submit: Leszek Swirski Commit-Queue: Camillo Bruni Reviewed-by: Camillo Bruni Cr-Commit-Position: refs/heads/main@{#83471} --- src/heap/factory-base.cc | 16 -- src/heap/factory-base.h | 5 - src/objects/hash-table-inl.h | 1 - src/objects/instance-type.h | 2 + src/objects/object-list-macros.h | 1 - src/objects/objects-body-descriptors-inl.h | 2 +- src/objects/objects-definitions.h | 2 + src/objects/objects-inl.h | 3 - src/objects/template-objects.cc | 78 ++---- src/objects/template-objects.h | 9 +- src/objects/template-objects.tq | 7 +- src/runtime/runtime-test.cc | 17 -- src/runtime/runtime.h | 2 - src/snapshot/context-serializer.cc | 11 +- test/mjsunit/mjsunit.js | 11 - test/mjsunit/regress/regress-1364429.js | 20 -- test/mjsunit/regress/regress-v8-13190.js | 48 ---- tools/v8heapconst.py | 302 ++++++++++----------- 18 files changed, 198 insertions(+), 339 deletions(-) delete mode 100644 test/mjsunit/regress/regress-1364429.js delete mode 100644 test/mjsunit/regress/regress-v8-13190.js diff --git a/src/heap/factory-base.cc b/src/heap/factory-base.cc index 32848668d6..0dec79f034 100644 --- a/src/heap/factory-base.cc +++ b/src/heap/factory-base.cc @@ -73,22 +73,6 @@ Handle FactoryBase::NewAccessorPair() { return handle(accessors, isolate()); } -template -Handle FactoryBase::NewCachedTemplateObject( - int function_literal_id, int slot_id, Handle next, - Handle template_object, AllocationType allocation) { - DCHECK(next->IsCachedTemplateObject() || next->IsTheHole()); - Map map = read_only_roots().cached_template_object_map(); - CachedTemplateObject result = CachedTemplateObject::cast( - AllocateRawWithImmortalMap(CachedTemplateObject::kSize, allocation, map)); - DisallowGarbageCollection no_gc; - result.set_function_literal_id(function_literal_id); - result.set_slot_id(slot_id); - result.set_template_object(HeapObjectReference::Weak(*template_object)); - result.set_next(*next); - return handle(result, isolate()); -} - template Handle FactoryBase::NewCodeDataContainer( int flags, AllocationType allocation) { diff --git a/src/heap/factory-base.h b/src/heap/factory-base.h index 2720071b6d..aea50e6cdb 100644 --- a/src/heap/factory-base.h +++ b/src/heap/factory-base.h @@ -102,11 +102,6 @@ class FactoryBase : public TorqueGeneratedFactory { Handle NewCodeDataContainer(int flags, AllocationType allocation); - Handle NewCachedTemplateObject( - int function_literal_id, int slot_id, Handle next, - Handle template_object, - AllocationType allocation_type = AllocationType::kYoung); - // Allocates a fixed array initialized with undefined values. Handle NewFixedArray( int length, AllocationType allocation = AllocationType::kYoung); diff --git a/src/objects/hash-table-inl.h b/src/objects/hash-table-inl.h index 311d2fac17..7ba51aa130 100644 --- a/src/objects/hash-table-inl.h +++ b/src/objects/hash-table-inl.h @@ -168,7 +168,6 @@ InternalIndex HashTable::FindEntry(PtrComprCageBase cage_base, uint32_t count = 1; Object undefined = roots.undefined_value(); Object the_hole = roots.the_hole_value(); - DCHECK_EQ(Shape::Hash(roots, key), static_cast(hash)); // EnsureCapacity will guarantee the hash table is never full. for (InternalIndex entry = FirstProbe(hash, capacity);; entry = NextProbe(entry, count++, capacity)) { diff --git a/src/objects/instance-type.h b/src/objects/instance-type.h index b7879b5a31..5726a1fc4f 100644 --- a/src/objects/instance-type.h +++ b/src/objects/instance-type.h @@ -315,6 +315,8 @@ IS_TYPE_FUNCTION_DECL(CodeT) V(_, BreakPointMap, break_point_map, BreakPoint) \ V(_, BreakPointInfoMap, break_point_info_map, BreakPointInfo) \ V(_, BytecodeArrayMap, bytecode_array_map, BytecodeArray) \ + V(_, CachedTemplateObjectMap, cached_template_object_map, \ + CachedTemplateObject) \ V(_, CellMap, cell_map, Cell) \ V(_, WeakCellMap, weak_cell_map, WeakCell) \ V(_, CodeMap, code_map, Code) \ diff --git a/src/objects/object-list-macros.h b/src/objects/object-list-macros.h index 7c6fe833d7..183c414f2e 100644 --- a/src/objects/object-list-macros.h +++ b/src/objects/object-list-macros.h @@ -91,7 +91,6 @@ class ZoneForwardList; V(CallHandlerInfo) \ V(Callable) \ V(Cell) \ - V(CachedTemplateObject) \ V(ClassBoilerplate) \ V(Code) \ V(CodeDataContainer) \ diff --git a/src/objects/objects-body-descriptors-inl.h b/src/objects/objects-body-descriptors-inl.h index 5b9eebf7ed..f8e83c1469 100644 --- a/src/objects/objects-body-descriptors-inl.h +++ b/src/objects/objects-body-descriptors-inl.h @@ -32,7 +32,7 @@ #include "src/objects/source-text-module.h" #include "src/objects/swiss-name-dictionary-inl.h" #include "src/objects/synthetic-module.h" -#include "src/objects/template-objects-inl.h" +#include "src/objects/template-objects.h" #include "src/objects/torque-defined-classes-inl.h" #include "src/objects/transitions.h" #include "src/objects/turbofan-types-inl.h" diff --git a/src/objects/objects-definitions.h b/src/objects/objects-definitions.h index 5bdc344604..75ce183310 100644 --- a/src/objects/objects-definitions.h +++ b/src/objects/objects-definitions.h @@ -152,6 +152,8 @@ namespace internal { async_generator_request) \ V(_, BREAK_POINT_TYPE, BreakPoint, break_point) \ V(_, BREAK_POINT_INFO_TYPE, BreakPointInfo, break_point_info) \ + V(_, CACHED_TEMPLATE_OBJECT_TYPE, CachedTemplateObject, \ + cached_template_object) \ V(_, CALL_SITE_INFO_TYPE, CallSiteInfo, call_site_info) \ V(_, CLASS_POSITIONS_TYPE, ClassPositions, class_positions) \ V(_, DEBUG_INFO_TYPE, DebugInfo, debug_info) \ diff --git a/src/objects/objects-inl.h b/src/objects/objects-inl.h index 816cc32719..e412f310e4 100644 --- a/src/objects/objects-inl.h +++ b/src/objects/objects-inl.h @@ -1140,9 +1140,6 @@ Object Object::GetSimpleHash(Object object) { } else if (InstanceTypeChecker::IsSharedFunctionInfo(instance_type)) { uint32_t hash = SharedFunctionInfo::cast(object).Hash(); return Smi::FromInt(hash & Smi::kMaxValue); - } else if (InstanceTypeChecker::IsScript(instance_type)) { - int id = Script::cast(object).id(); - return Smi::FromInt(ComputeUnseededHash(id) & Smi::kMaxValue); } DCHECK(object.IsJSReceiver()); return object; diff --git a/src/objects/template-objects.cc b/src/objects/template-objects.cc index 3b1bc627ab..12eb1d18ee 100644 --- a/src/objects/template-objects.cc +++ b/src/objects/template-objects.cc @@ -19,57 +19,26 @@ Handle TemplateObjectDescription::GetTemplateObject( Isolate* isolate, Handle native_context, Handle description, Handle shared_info, int slot_id) { - int function_literal_id = shared_info->function_literal_id(); + uint32_t hash = shared_info->Hash(); // Check the template weakmap to see if the template object already exists. Handle template_weakmap; - Handle