From 3cfcc7e111a32f231a483b8feda19ef2d8c45253 Mon Sep 17 00:00:00 2001 From: mvstanton Date: Tue, 7 Jun 2016 05:01:51 -0700 Subject: [PATCH] Avoid creating weak cells for literal arrays that are empty of literals. It may be that we have a feedback vector, but no literals. In this case we can store into the OptimizedCodeMap directly instead of using a WeakCell, because all data in the feedback vector is already held weakly. The use of a WeakCell in the OptimizedCodeMap is only required when there are literals which may hold maps strongly. This is to address a performance regression caused by the creation of a large number of WeakCells. BUG=chromium:615831 Review-Url: https://codereview.chromium.org/2031123003 Cr-Commit-Position: refs/heads/master@{#36786} --- src/arm/builtins-arm.cc | 18 ++++++++++++- src/arm64/builtins-arm64.cc | 20 +++++++++++++- src/ia32/builtins-ia32.cc | 19 +++++++++++++- src/mips/builtins-mips.cc | 17 +++++++++++- src/mips64/builtins-mips64.cc | 16 +++++++++++- src/objects.cc | 49 +++++++++++++++++++++++++---------- src/type-feedback-vector.cc | 18 +++++++++---- src/x64/builtins-x64.cc | 19 +++++++++++++- 8 files changed, 152 insertions(+), 24 deletions(-) diff --git a/src/arm/builtins-arm.cc b/src/arm/builtins-arm.cc index 665993bafc..ad1a064600 100644 --- a/src/arm/builtins-arm.cc +++ b/src/arm/builtins-arm.cc @@ -1297,13 +1297,29 @@ void Builtins::Generate_CompileLazy(MacroAssembler* masm) { const int bailout_id = BailoutId::None().ToInt(); __ cmp(temp, Operand(Smi::FromInt(bailout_id))); __ b(ne, &loop_bottom); + // Literals available? + Label got_literals, maybe_cleared_weakcell; __ ldr(temp, FieldMemOperand(array_pointer, SharedFunctionInfo::kOffsetToPreviousLiterals)); + // temp contains either a WeakCell pointing to the literals array or the + // literals array directly. + STATIC_ASSERT(WeakCell::kValueOffset == FixedArray::kLengthOffset); + __ ldr(r4, FieldMemOperand(temp, WeakCell::kValueOffset)); + __ JumpIfSmi(r4, &maybe_cleared_weakcell); + // r4 is a pointer, therefore temp is a WeakCell pointing to a literals array. __ ldr(temp, FieldMemOperand(temp, WeakCell::kValueOffset)); - __ JumpIfSmi(temp, &gotta_call_runtime); + __ jmp(&got_literals); + + // r4 is a smi. If it's 0, then we are looking at a cleared WeakCell + // around the literals array, and we should visit the runtime. If it's > 0, + // then temp already contains the literals array. + __ bind(&maybe_cleared_weakcell); + __ cmp(r4, Operand(Smi::FromInt(0))); + __ b(eq, &gotta_call_runtime); // Save the literals in the closure. + __ bind(&got_literals); __ ldr(r4, MemOperand(sp, 0)); __ str(temp, FieldMemOperand(r4, JSFunction::kLiteralsOffset)); __ push(index); diff --git a/src/arm64/builtins-arm64.cc b/src/arm64/builtins-arm64.cc index f17a3001dd..4e69404e02 100644 --- a/src/arm64/builtins-arm64.cc +++ b/src/arm64/builtins-arm64.cc @@ -1308,13 +1308,31 @@ void Builtins::Generate_CompileLazy(MacroAssembler* masm) { const int bailout_id = BailoutId::None().ToInt(); __ Cmp(temp, Operand(Smi::FromInt(bailout_id))); __ B(ne, &loop_bottom); + // Literals available? + Label got_literals, maybe_cleared_weakcell; + Register temp2 = x7; __ Ldr(temp, FieldMemOperand(array_pointer, SharedFunctionInfo::kOffsetToPreviousLiterals)); + // temp contains either a WeakCell pointing to the literals array or the + // literals array directly. + STATIC_ASSERT(WeakCell::kValueOffset == FixedArray::kLengthOffset); + __ Ldr(temp2, FieldMemOperand(temp, WeakCell::kValueOffset)); + __ JumpIfSmi(temp2, &maybe_cleared_weakcell); + // temp2 is a pointer, therefore temp is a WeakCell pointing to a literals + // array. __ Ldr(temp, FieldMemOperand(temp, WeakCell::kValueOffset)); - __ JumpIfSmi(temp, &gotta_call_runtime); + __ jmp(&got_literals); + + // r4 is a smi. If it's 0, then we are looking at a cleared WeakCell + // around the literals array, and we should visit the runtime. If it's > 0, + // then temp already contains the literals array. + __ bind(&maybe_cleared_weakcell); + __ Cmp(temp2, Operand(Smi::FromInt(0))); + __ B(eq, &gotta_call_runtime); // Save the literals in the closure. + __ bind(&got_literals); __ Str(temp, FieldMemOperand(closure, JSFunction::kLiteralsOffset)); __ RecordWriteField(closure, JSFunction::kLiteralsOffset, temp, x7, kLRHasNotBeenSaved, kDontSaveFPRegs, EMIT_REMEMBERED_SET, diff --git a/src/ia32/builtins-ia32.cc b/src/ia32/builtins-ia32.cc index 4178067930..db558a7f55 100644 --- a/src/ia32/builtins-ia32.cc +++ b/src/ia32/builtins-ia32.cc @@ -896,13 +896,30 @@ void Builtins::Generate_CompileLazy(MacroAssembler* masm) { const int bailout_id = BailoutId::None().ToInt(); __ cmp(temp, Immediate(Smi::FromInt(bailout_id))); __ j(not_equal, &loop_bottom); + // Literals available? + Label got_literals, maybe_cleared_weakcell; __ mov(temp, FieldOperand(map, index, times_half_pointer_size, SharedFunctionInfo::kOffsetToPreviousLiterals)); + + // temp contains either a WeakCell pointing to the literals array or the + // literals array directly. + STATIC_ASSERT(WeakCell::kValueOffset == FixedArray::kLengthOffset); + __ JumpIfSmi(FieldOperand(temp, WeakCell::kValueOffset), + &maybe_cleared_weakcell); + // The WeakCell value is a pointer, therefore it's a valid literals array. __ mov(temp, FieldOperand(temp, WeakCell::kValueOffset)); - __ JumpIfSmi(temp, &gotta_call_runtime); + __ jmp(&got_literals); + + // We have a smi. If it's 0, then we are looking at a cleared WeakCell + // around the literals array, and we should visit the runtime. If it's > 0, + // then temp already contains the literals array. + __ bind(&maybe_cleared_weakcell); + __ cmp(FieldOperand(temp, WeakCell::kValueOffset), Immediate(0)); + __ j(equal, &gotta_call_runtime); // Save the literals in the closure. + __ bind(&got_literals); __ mov(ecx, Operand(esp, 0)); __ mov(FieldOperand(ecx, JSFunction::kLiteralsOffset), temp); __ push(index); diff --git a/src/mips/builtins-mips.cc b/src/mips/builtins-mips.cc index 19de5d953f..68d9570d4f 100644 --- a/src/mips/builtins-mips.cc +++ b/src/mips/builtins-mips.cc @@ -1294,13 +1294,28 @@ void Builtins::Generate_CompileLazy(MacroAssembler* masm) { SharedFunctionInfo::kOffsetToPreviousOsrAstId)); const int bailout_id = BailoutId::None().ToInt(); __ Branch(&loop_bottom, ne, temp, Operand(Smi::FromInt(bailout_id))); + // Literals available? + Label got_literals, maybe_cleared_weakcell; __ lw(temp, FieldMemOperand(array_pointer, SharedFunctionInfo::kOffsetToPreviousLiterals)); + // temp contains either a WeakCell pointing to the literals array or the + // literals array directly. + STATIC_ASSERT(WeakCell::kValueOffset == FixedArray::kLengthOffset); + __ lw(t0, FieldMemOperand(temp, WeakCell::kValueOffset)); + __ JumpIfSmi(t0, &maybe_cleared_weakcell); + // t0 is a pointer, therefore temp is a WeakCell pointing to a literals array. __ lw(temp, FieldMemOperand(temp, WeakCell::kValueOffset)); - __ JumpIfSmi(temp, &gotta_call_runtime); + __ jmp(&got_literals); + + // t0 is a smi. If it's 0, then we are looking at a cleared WeakCell + // around the literals array, and we should visit the runtime. If it's > 0, + // then temp already contains the literals array. + __ bind(&maybe_cleared_weakcell); + __ Branch(&gotta_call_runtime, eq, t0, Operand(Smi::FromInt(0))); // Save the literals in the closure. + __ bind(&got_literals); __ lw(t0, MemOperand(sp, 0)); __ sw(temp, FieldMemOperand(t0, JSFunction::kLiteralsOffset)); __ push(index); diff --git a/src/mips64/builtins-mips64.cc b/src/mips64/builtins-mips64.cc index 278811c1e8..c4ae7edee7 100644 --- a/src/mips64/builtins-mips64.cc +++ b/src/mips64/builtins-mips64.cc @@ -1280,13 +1280,27 @@ void Builtins::Generate_CompileLazy(MacroAssembler* masm) { SharedFunctionInfo::kOffsetToPreviousOsrAstId)); const int bailout_id = BailoutId::None().ToInt(); __ Branch(&loop_bottom, ne, temp, Operand(Smi::FromInt(bailout_id))); + // Literals available? + Label got_literals, maybe_cleared_weakcell; __ ld(temp, FieldMemOperand(array_pointer, SharedFunctionInfo::kOffsetToPreviousLiterals)); + // temp contains either a WeakCell pointing to the literals array or the + // literals array directly. + __ ld(a4, FieldMemOperand(temp, WeakCell::kValueOffset)); + __ JumpIfSmi(a4, &maybe_cleared_weakcell); + // a4 is a pointer, therefore temp is a WeakCell pointing to a literals array. __ ld(temp, FieldMemOperand(temp, WeakCell::kValueOffset)); - __ JumpIfSmi(temp, &gotta_call_runtime); + __ jmp(&got_literals); + + // a4 is a smi. If it's 0, then we are looking at a cleared WeakCell + // around the literals array, and we should visit the runtime. If it's > 0, + // then temp already contains the literals array. + __ bind(&maybe_cleared_weakcell); + __ Branch(&gotta_call_runtime, eq, a4, Operand(Smi::FromInt(0))); // Save the literals in the closure. + __ bind(&got_literals); __ ld(a4, MemOperand(sp, 0)); __ sd(temp, FieldMemOperand(a4, JSFunction::kLiteralsOffset)); __ push(index); diff --git a/src/objects.cc b/src/objects.cc index 171a95bcba..adce80f92c 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -11610,9 +11610,13 @@ void SharedFunctionInfo::AddToOptimizedCodeMap( isolate->factory()->NewWeakCell(code.ToHandleChecked()); old_code_map->set(entry + kCachedCodeOffset, *code_cell); } - Handle literals_cell = - isolate->factory()->NewWeakCell(literals); - old_code_map->set(entry + kLiteralsOffset, *literals_cell); + if (literals->literals_count() == 0) { + old_code_map->set(entry + kLiteralsOffset, *literals); + } else { + Handle literals_cell = + isolate->factory()->NewWeakCell(literals); + old_code_map->set(entry + kLiteralsOffset, *literals_cell); + } return; } @@ -11643,12 +11647,18 @@ void SharedFunctionInfo::AddToOptimizedCodeMap( Handle code_cell = code.is_null() ? isolate->factory()->empty_weak_cell() : isolate->factory()->NewWeakCell(code.ToHandleChecked()); - Handle literals_cell = isolate->factory()->NewWeakCell(literals); WeakCell* context_cell = native_context->self_weak_cell(); new_code_map->set(entry + kContextOffset, context_cell); new_code_map->set(entry + kCachedCodeOffset, *code_cell); - new_code_map->set(entry + kLiteralsOffset, *literals_cell); + + if (literals->literals_count() == 0) { + new_code_map->set(entry + kLiteralsOffset, *literals); + } else { + Handle literals_cell = isolate->factory()->NewWeakCell(literals); + new_code_map->set(entry + kLiteralsOffset, *literals_cell); + } + new_code_map->set(entry + kOsrAstIdOffset, Smi::FromInt(osr_ast_id.ToInt())); #ifdef DEBUG @@ -11659,8 +11669,16 @@ void SharedFunctionInfo::AddToOptimizedCodeMap( DCHECK(cell->cleared() || (cell->value()->IsCode() && Code::cast(cell->value())->kind() == Code::OPTIMIZED_FUNCTION)); - cell = WeakCell::cast(new_code_map->get(i + kLiteralsOffset)); - DCHECK(cell->cleared() || cell->value()->IsFixedArray()); + Object* lits = new_code_map->get(i + kLiteralsOffset); + if (lits->IsWeakCell()) { + cell = WeakCell::cast(lits); + DCHECK(cell->cleared() || + (cell->value()->IsLiteralsArray() && + LiteralsArray::cast(cell->value())->literals_count() > 0)); + } else { + DCHECK(lits->IsLiteralsArray() && + LiteralsArray::cast(lits)->literals_count() == 0); + } DCHECK(new_code_map->get(i + kOsrAstIdOffset)->IsSmi()); } #endif @@ -13278,13 +13296,18 @@ CodeAndLiterals SharedFunctionInfo::SearchOptimizedCodeMap( } else { DCHECK_LE(entry + kEntryLength, code_map->length()); WeakCell* cell = WeakCell::cast(code_map->get(entry + kCachedCodeOffset)); - WeakCell* literals_cell = - WeakCell::cast(code_map->get(entry + kLiteralsOffset)); - + Object* lits = code_map->get(entry + kLiteralsOffset); + LiteralsArray* literals = nullptr; + if (lits->IsWeakCell()) { + WeakCell* literal_cell = WeakCell::cast(lits); + if (!literal_cell->cleared()) { + literals = LiteralsArray::cast(literal_cell->value()); + } + } else { + literals = LiteralsArray::cast(lits); + } result = {cell->cleared() ? nullptr : Code::cast(cell->value()), - literals_cell->cleared() - ? nullptr - : LiteralsArray::cast(literals_cell->value())}; + literals}; } } return result; diff --git a/src/type-feedback-vector.cc b/src/type-feedback-vector.cc index 789ac2ea77..723143db22 100644 --- a/src/type-feedback-vector.cc +++ b/src/type-feedback-vector.cc @@ -284,11 +284,19 @@ void TypeFeedbackVector::ClearAllKeyedStoreICs(Isolate* isolate) { int length = optimized_code_map->length(); for (int i = SharedFunctionInfo::kEntriesStart; i < length; i += SharedFunctionInfo::kEntryLength) { - WeakCell* cell = WeakCell::cast( - optimized_code_map->get(i + SharedFunctionInfo::kLiteralsOffset)); - if (cell->value()->IsLiteralsArray()) { - TypeFeedbackVector* vector = - LiteralsArray::cast(cell->value())->feedback_vector(); + Object* lits = + optimized_code_map->get(i + SharedFunctionInfo::kLiteralsOffset); + TypeFeedbackVector* vector = nullptr; + if (lits->IsWeakCell()) { + WeakCell* cell = WeakCell::cast(lits); + if (cell->value()->IsLiteralsArray()) { + vector = LiteralsArray::cast(cell->value())->feedback_vector(); + } + } else { + DCHECK(lits->IsLiteralsArray()); + vector = LiteralsArray::cast(lits)->feedback_vector(); + } + if (vector != nullptr) { vector->ClearKeyedStoreICs(shared); } } diff --git a/src/x64/builtins-x64.cc b/src/x64/builtins-x64.cc index e7a567527d..ac41762cea 100644 --- a/src/x64/builtins-x64.cc +++ b/src/x64/builtins-x64.cc @@ -962,13 +962,30 @@ void Builtins::Generate_CompileLazy(MacroAssembler* masm) { const int bailout_id = BailoutId::None().ToInt(); __ cmpl(temp, Immediate(bailout_id)); __ j(not_equal, &loop_bottom); + // Literals available? + Label got_literals, maybe_cleared_weakcell; __ movp(temp, FieldOperand(map, index, times_pointer_size, SharedFunctionInfo::kOffsetToPreviousLiterals)); + // temp contains either a WeakCell pointing to the literals array or the + // literals array directly. + STATIC_ASSERT(WeakCell::kValueOffset == FixedArray::kLengthOffset); + __ movp(r15, FieldOperand(temp, WeakCell::kValueOffset)); + __ JumpIfSmi(r15, &maybe_cleared_weakcell); + // r15 is a pointer, therefore temp is a WeakCell pointing to a literals + // array. __ movp(temp, FieldOperand(temp, WeakCell::kValueOffset)); - __ JumpIfSmi(temp, &gotta_call_runtime); + __ jmp(&got_literals); + + // r15 is a smi. If it's 0, then we are looking at a cleared WeakCell + // around the literals array, and we should visit the runtime. If it's > 0, + // then temp already contains the literals array. + __ bind(&maybe_cleared_weakcell); + __ cmpp(r15, Immediate(0)); + __ j(equal, &gotta_call_runtime); // Save the literals in the closure. + __ bind(&got_literals); __ movp(FieldOperand(closure, JSFunction::kLiteralsOffset), temp); __ movp(r15, index); __ RecordWriteField(closure, JSFunction::kLiteralsOffset, temp, r15,