From be1164775f0f75a2711e767d7041c73a7f701288 Mon Sep 17 00:00:00 2001 From: Victor Gomes Date: Mon, 23 Sep 2019 15:51:51 +0200 Subject: [PATCH] Reland "[Context] Add a bit flag to indicate if extension might exist" This is a reland of d7b67ce2068cf5e429b0ebf683436276514f3a6f Original change's description: > [Context] Add a bit flag to indicate if extension might exist > > Checking the bit flag instead of comparing pointers should improve performance. > This will also allow us to remove the extension slot in Context and save memory. > > Bug: v8:9744 > Change-Id: I7ab9feeadfb934955798d877d13bc0e1d78a191c > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1814918 > Commit-Queue: Victor Gomes > Reviewed-by: Ulan Degenbaev > Reviewed-by: Leszek Swirski > Cr-Commit-Position: refs/heads/master@{#63906} Bug: v8:9744 Change-Id: Ic4725ad5730a8f8fff6288d6af2205c230aff79d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1815256 Reviewed-by: Leszek Swirski Reviewed-by: Ulan Degenbaev Commit-Queue: Victor Gomes Cr-Commit-Position: refs/heads/master@{#63993} --- src/codegen/code-stub-assembler.cc | 7 +++++ src/codegen/code-stub-assembler.h | 1 + src/heap/factory.cc | 2 +- src/interpreter/interpreter-assembler.cc | 29 +++++++++++--------- src/objects/contexts-inl.h | 34 +++++++++++++++++++++--- src/objects/contexts.h | 14 +++++++++- src/objects/objects.cc | 3 ++- 7 files changed, 72 insertions(+), 18 deletions(-) diff --git a/src/codegen/code-stub-assembler.cc b/src/codegen/code-stub-assembler.cc index 4e8f718f03..becfb24b4a 100644 --- a/src/codegen/code-stub-assembler.cc +++ b/src/codegen/code-stub-assembler.cc @@ -2717,6 +2717,13 @@ TNode CodeStubAssembler::LoadDoubleWithHoleCheck( return UncheckedCast(Load(machine_type, base, offset)); } +TNode CodeStubAssembler::LoadContextHasExtensionField( + SloppyTNode context) { + TNode value = + LoadAndUntagObjectField(context, Context::kLengthOffset); + return IsSetWord(value); +} + TNode CodeStubAssembler::LoadContextElement( SloppyTNode context, int slot_index) { int offset = Context::SlotOffset(slot_index); diff --git a/src/codegen/code-stub-assembler.h b/src/codegen/code-stub-assembler.h index 1804c557c6..18bedbbaa8 100644 --- a/src/codegen/code-stub-assembler.h +++ b/src/codegen/code-stub-assembler.h @@ -1433,6 +1433,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler TNode BigIntFromUint32Pair(TNode low, TNode high); // Context manipulation + TNode LoadContextHasExtensionField(SloppyTNode context); TNode LoadContextElement(SloppyTNode context, int slot_index); TNode LoadContextElement(SloppyTNode context, diff --git a/src/heap/factory.cc b/src/heap/factory.cc index fdfbaf682f..ab5c818cdd 100644 --- a/src/heap/factory.cc +++ b/src/heap/factory.cc @@ -1422,7 +1422,7 @@ Handle Factory::NewContext(RootIndex map_root_index, int size, Map map = Map::cast(isolate()->root(map_root_index)); HeapObject result = AllocateRawWithImmortalMap(size, allocation, map); Handle context(Context::cast(result), isolate()); - context->set_length(variadic_part_length); + context->initialize_length_and_extension_bit(variadic_part_length); DCHECK_EQ(context->SizeFromMap(map), size); if (size > Context::kTodoHeaderSize) { ObjectSlot start = context->RawField(Context::kTodoHeaderSize); diff --git a/src/interpreter/interpreter-assembler.cc b/src/interpreter/interpreter-assembler.cc index 6d316f2644..16ebab9956 100644 --- a/src/interpreter/interpreter-assembler.cc +++ b/src/interpreter/interpreter-assembler.cc @@ -205,27 +205,32 @@ void InterpreterAssembler::GotoIfHasContextExtensionUpToDepth( TVARIABLE(Uint32T, cur_depth, depth); Label context_search(this, {&cur_depth, &cur_context}); + Label no_extension(this); // Loop until the depth is 0. Goto(&context_search); BIND(&context_search); { - // TODO(leszeks): We only need to do this check if the context had a sloppy - // eval, we could pass in a context chain bitmask to figure out which - // contexts actually need to be checked. - - TNode extension_slot = - LoadContextElement(cur_context.value(), Context::EXTENSION_INDEX); + // Check if context has an extension slot + TNode has_extension = + LoadContextHasExtensionField(cur_context.value()); + GotoIfNot(has_extension, &no_extension); // Jump to the target if the extension slot is not a hole. - GotoIf(TaggedNotEqual(extension_slot, TheHoleConstant()), target); + TNode extension_slot = + LoadContextElement(cur_context.value(), Context::EXTENSION_INDEX); + Branch(TaggedNotEqual(extension_slot, TheHoleConstant()), target, + &no_extension); - cur_depth = Unsigned(Int32Sub(cur_depth.value(), Int32Constant(1))); - cur_context = - CAST(LoadContextElement(cur_context.value(), Context::PREVIOUS_INDEX)); + BIND(&no_extension); + { + cur_depth = Unsigned(Int32Sub(cur_depth.value(), Int32Constant(1))); + cur_context = CAST( + LoadContextElement(cur_context.value(), Context::PREVIOUS_INDEX)); - GotoIf(Word32NotEqual(cur_depth.value(), Int32Constant(0)), - &context_search); + GotoIf(Word32NotEqual(cur_depth.value(), Int32Constant(0)), + &context_search); + } } } diff --git a/src/objects/contexts-inl.h b/src/objects/contexts-inl.h index 7c232e288c..d883dd63a3 100644 --- a/src/objects/contexts-inl.h +++ b/src/objects/contexts-inl.h @@ -47,10 +47,29 @@ Context ScriptContextTable::get_context(int i) const { OBJECT_CONSTRUCTORS_IMPL(Context, HeapObject) NEVER_READ_ONLY_SPACE_IMPL(Context) CAST_ACCESSOR(Context) -SMI_ACCESSORS(Context, length, kLengthOffset) + +SMI_ACCESSORS(Context, length_and_extension_flag, kLengthOffset) +SYNCHRONIZED_SMI_ACCESSORS(Context, length_and_extension_flag, kLengthOffset) CAST_ACCESSOR(NativeContext) +int Context::length() const { + return LengthField::decode(length_and_extension_flag()); +} + +int Context::synchronized_length() const { + return LengthField::decode(synchronized_length_and_extension_flag()); +} + +void Context::initialize_length_and_extension_bit(int len, + Context::HasExtension flag) { + DCHECK(LengthField::is_valid(len)); + int value = 0; + value = LengthField::update(value, len); + value = HasExtensionField::update(value, flag == Context::HasExtension::kYes); + set_length_and_extension_flag(value); +} + Object Context::get(int index) const { Isolate* isolate = GetIsolateForPtrCompr(*this); return get(isolate, index); @@ -94,11 +113,20 @@ void Context::set_previous(Context context) { set(PREVIOUS_INDEX, context); } Object Context::next_context_link() { return get(Context::NEXT_CONTEXT_LINK); } -bool Context::has_extension() { return !extension().IsTheHole(); } +bool Context::has_extension() { + return static_cast( + HasExtensionField::decode(length_and_extension_flag())) && + !extension().IsTheHole(); +} + HeapObject Context::extension() { return HeapObject::cast(get(EXTENSION_INDEX)); } -void Context::set_extension(HeapObject object) { set(EXTENSION_INDEX, object); } +void Context::set_extension(HeapObject object) { + set(EXTENSION_INDEX, object); + synchronized_set_length_and_extension_flag( + HasExtensionField::update(length_and_extension_flag(), true)); +} NativeContext Context::native_context() const { Object result = get(NATIVE_CONTEXT_INDEX); diff --git a/src/objects/contexts.h b/src/objects/contexts.h index ebfb1b613b..fd82747cdf 100644 --- a/src/objects/contexts.h +++ b/src/objects/contexts.h @@ -453,9 +453,19 @@ class Context : public HeapObject { DECL_CAST(Context) + enum class HasExtension { kYes, kNo }; + // [length]: length of the context. V8_INLINE int length() const; - V8_INLINE void set_length(int value); + V8_INLINE int synchronized_length() const; + V8_INLINE void initialize_length_and_extension_bit( + int len, HasExtension flag = HasExtension::kNo); + + // We use the 30th bit. Otherwise if we set the 31st bit, + // the number would be pottentially bigger than an SMI. + // Any DCHECK(Smi::IsValue(...)) would fail. + using LengthField = BitField; + using HasExtensionField = BitField; // Setter and getter for elements. V8_INLINE Object get(int index) const; @@ -662,6 +672,8 @@ class Context : public HeapObject { #endif OBJECT_CONSTRUCTORS(Context, HeapObject); + DECL_INT_ACCESSORS(length_and_extension_flag) + DECL_SYNCHRONIZED_INT_ACCESSORS(length_and_extension_flag) }; class NativeContext : public Context { diff --git a/src/objects/objects.cc b/src/objects/objects.cc index 0cd74334d8..7d4f66cc32 100644 --- a/src/objects/objects.cc +++ b/src/objects/objects.cc @@ -2188,7 +2188,8 @@ int HeapObject::SizeFromMap(Map map) const { } if (IsInRange(instance_type, FIRST_CONTEXT_TYPE, LAST_CONTEXT_TYPE)) { if (instance_type == NATIVE_CONTEXT_TYPE) return NativeContext::kSize; - return Context::SizeFor(Context::unchecked_cast(*this).length()); + return Context::SizeFor( + Context::unchecked_cast(*this).synchronized_length()); } if (instance_type == ONE_BYTE_STRING_TYPE || instance_type == ONE_BYTE_INTERNALIZED_STRING_TYPE) {