Skip to content

Commit

Permalink
[vm, compiler] Remove quadratic time spent verifying handle lifetime.
Browse files Browse the repository at this point in the history
Instead of checking whether a handle is a zone handle rather than a scoped handle by iterating the zone handle blocks, reserve an extra slot in debug mode to record the handle's lifetime.

TEST=ci
Bug: #50408
Change-Id: I121c76d02b03e5d2141450077bc6e9da3cb60a5d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/268801
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
  • Loading branch information
rmacnak-google authored and Commit Queue committed Nov 11, 2022
1 parent 39ec46e commit 715866a
Show file tree
Hide file tree
Showing 30 changed files with 109 additions and 81 deletions.
2 changes: 1 addition & 1 deletion runtime/vm/code_descriptors.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class ExceptionHandlerList : public ZoneAllocated {
ASSERT(list_[try_index].pc_offset == ExceptionHandlers::kInvalidPcOffset);
list_[try_index].pc_offset = pc_offset;
list_[try_index].is_generated = is_generated;
ASSERT(handler_types.IsNotTemporaryScopedHandle());
DEBUG_ASSERT(handler_types.IsNotTemporaryScopedHandle());
list_[try_index].handler_types = &handler_types;
list_[try_index].needs_stacktrace |= needs_stacktrace;
}
Expand Down
4 changes: 2 additions & 2 deletions runtime/vm/compiler/assembler/assembler_arm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1631,7 +1631,7 @@ bool Assembler::CanLoadFromObjectPool(const Object& object) const {
return false;
}

ASSERT(IsNotTemporaryScopedHandle(object));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(object));
ASSERT(IsInOldSpace(object));
return true;
}
Expand Down Expand Up @@ -1901,7 +1901,7 @@ void Assembler::StoreIntoObjectNoBarrier(Register object,
const Object& value,
MemoryOrder memory_order) {
ASSERT(IsOriginalObject(value));
ASSERT(IsNotTemporaryScopedHandle(value));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(value));
// No store buffer update.
LoadObject(IP, value);
if (memory_order == kRelease) {
Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/compiler/assembler/assembler_arm64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ bool Assembler::CanLoadFromObjectPool(const Object& object) const {
return false;
}

ASSERT(IsNotTemporaryScopedHandle(object));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(object));
ASSERT(IsInOldSpace(object));
return true;
}
Expand Down Expand Up @@ -1255,7 +1255,7 @@ void Assembler::StoreIntoObjectNoBarrier(Register object,
MemoryOrder memory_order) {
RELEASE_ASSERT(memory_order == kRelaxedNonAtomic);
ASSERT(IsOriginalObject(value));
ASSERT(IsNotTemporaryScopedHandle(value));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(value));
if (IsSameObject(compiler::NullObject(), value)) {
str(NULL_REG, dest);
} else if (target::IsSmi(value) && (target::ToRawSmi(value) == 0)) {
Expand All @@ -1273,7 +1273,7 @@ void Assembler::StoreCompressedIntoObjectNoBarrier(Register object,
// stlr does not feature an address operand.
RELEASE_ASSERT(memory_order == kRelaxedNonAtomic);
ASSERT(IsOriginalObject(value));
ASSERT(IsNotTemporaryScopedHandle(value));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(value));
// No store buffer update.
if (IsSameObject(compiler::NullObject(), value)) {
str(NULL_REG, dest, kObjectBytes);
Expand Down
12 changes: 6 additions & 6 deletions runtime/vm/compiler/assembler/assembler_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ intptr_t AssemblerBuffer::CountPointerOffsets() const {
void AssemblerBuffer::EmitObject(const Object& object) {
// Since we are going to store the handle as part of the fixup information
// the handle needs to be a zone handle.
ASSERT(IsNotTemporaryScopedHandle(object));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(object));
ASSERT(IsInOldSpace(object));
EmitFixup(new PatchCodeWithHandle(pointer_offsets_, object));
cursor_ += target::kWordSize; // Reserve space for pointer.
Expand Down Expand Up @@ -346,7 +346,7 @@ void ObjectPoolBuilder::Reset() {
intptr_t ObjectPoolBuilder::AddObject(
const Object& obj,
ObjectPoolBuilderEntry::Patchability patchable) {
ASSERT(IsNotTemporaryScopedHandle(obj));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(obj));
return AddObject(ObjectPoolBuilderEntry(&obj, patchable));
}

Expand All @@ -373,10 +373,10 @@ intptr_t ObjectPoolBuilder::AddImmediate128(simd128_value_t imm) {
}

intptr_t ObjectPoolBuilder::AddObject(ObjectPoolBuilderEntry entry) {
ASSERT((entry.type() != ObjectPoolBuilderEntry::kTaggedObject) ||
(IsNotTemporaryScopedHandle(*entry.obj_) &&
(entry.equivalence_ == NULL ||
IsNotTemporaryScopedHandle(*entry.equivalence_))));
DEBUG_ASSERT((entry.type() != ObjectPoolBuilderEntry::kTaggedObject) ||
(IsNotTemporaryScopedHandle(*entry.obj_) &&
(entry.equivalence_ == NULL ||
IsNotTemporaryScopedHandle(*entry.equivalence_))));

if (entry.type() == ObjectPoolBuilderEntry::kTaggedObject) {
// If the owner of the object pool wrapper specified a specific zone we
Expand Down
8 changes: 4 additions & 4 deletions runtime/vm/compiler/assembler/assembler_ia32.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1971,7 +1971,7 @@ void Assembler::LoadObject(Register dst,
!movable_referent) {
movl(dst, Immediate(target::ToRawPointer(object)));
} else {
ASSERT(IsNotTemporaryScopedHandle(object));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(object));
ASSERT(IsInOldSpace(object));
AssemblerBuffer::EnsureCapacity ensured(&buffer_);
EmitUint8(0xB8 + dst);
Expand All @@ -1995,7 +1995,7 @@ void Assembler::PushObject(const Object& object) {
if (target::CanEmbedAsRawPointerInGeneratedCode(object)) {
pushl(Immediate(target::ToRawPointer(object)));
} else {
ASSERT(IsNotTemporaryScopedHandle(object));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(object));
ASSERT(IsInOldSpace(object));
AssemblerBuffer::EnsureCapacity ensured(&buffer_);
EmitUint8(0x68);
Expand All @@ -2008,7 +2008,7 @@ void Assembler::CompareObject(Register reg, const Object& object) {
if (target::CanEmbedAsRawPointerInGeneratedCode(object)) {
cmpl(reg, Immediate(target::ToRawPointer(object)));
} else {
ASSERT(IsNotTemporaryScopedHandle(object));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(object));
ASSERT(IsInOldSpace(object));
AssemblerBuffer::EnsureCapacity ensured(&buffer_);
if (reg == EAX) {
Expand Down Expand Up @@ -2757,7 +2757,7 @@ void Assembler::CopyMemoryWords(Register src,
}

void Assembler::PushCodeObject() {
ASSERT(IsNotTemporaryScopedHandle(code_));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(code_));
AssemblerBuffer::EnsureCapacity ensured(&buffer_);
EmitUint8(0x68);
buffer_.EmitObject(code_);
Expand Down
4 changes: 2 additions & 2 deletions runtime/vm/compiler/assembler/assembler_riscv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3200,7 +3200,7 @@ void Assembler::StoreIntoObjectNoBarrier(Register object,
const Object& value,
MemoryOrder memory_order) {
ASSERT(IsOriginalObject(value));
ASSERT(IsNotTemporaryScopedHandle(value));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(value));
// No store buffer update.
Register value_reg;
if (IsSameObject(compiler::NullObject(), value)) {
Expand Down Expand Up @@ -3279,7 +3279,7 @@ bool Assembler::CanLoadFromObjectPool(const Object& object) const {
return false;
}

ASSERT(IsNotTemporaryScopedHandle(object));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(object));
ASSERT(IsInOldSpace(object));
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/compiler/assembler/assembler_x64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,7 @@ bool Assembler::CanLoadFromObjectPool(const Object& object) const {
return false;
}

ASSERT(IsNotTemporaryScopedHandle(object));
DEBUG_ASSERT(IsNotTemporaryScopedHandle(object));
ASSERT(IsInOldSpace(object));
return true;
}
Expand Down
12 changes: 6 additions & 6 deletions runtime/vm/compiler/backend/flow_graph_compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,7 @@ void FlowGraphCompiler::AddNullCheck(const InstructionSource& source,

void FlowGraphCompiler::AddPcRelativeCallTarget(const Function& function,
Code::EntryKind entry_kind) {
ASSERT(function.IsZoneHandle());
DEBUG_ASSERT(function.IsNotTemporaryScopedHandle());
const auto entry_point = entry_kind == Code::EntryKind::kUnchecked
? Code::kUncheckedEntry
: Code::kDefaultEntry;
Expand All @@ -941,15 +941,15 @@ void FlowGraphCompiler::AddPcRelativeCallTarget(const Function& function,
}

void FlowGraphCompiler::AddPcRelativeCallStubTarget(const Code& stub_code) {
ASSERT(stub_code.IsZoneHandle() || stub_code.IsReadOnlyHandle());
DEBUG_ASSERT(stub_code.IsNotTemporaryScopedHandle());
ASSERT(!stub_code.IsNull());
static_calls_target_table_.Add(new (zone()) StaticCallsStruct(
Code::kPcRelativeCall, Code::kDefaultEntry, assembler()->CodeSize(),
nullptr, &stub_code, nullptr));
}

void FlowGraphCompiler::AddPcRelativeTailCallStubTarget(const Code& stub_code) {
ASSERT(stub_code.IsZoneHandle() || stub_code.IsReadOnlyHandle());
DEBUG_ASSERT(stub_code.IsNotTemporaryScopedHandle());
ASSERT(!stub_code.IsNull());
static_calls_target_table_.Add(new (zone()) StaticCallsStruct(
Code::kPcRelativeTailCall, Code::kDefaultEntry, assembler()->CodeSize(),
Expand All @@ -958,7 +958,7 @@ void FlowGraphCompiler::AddPcRelativeTailCallStubTarget(const Code& stub_code) {

void FlowGraphCompiler::AddPcRelativeTTSCallTypeTarget(
const AbstractType& dst_type) {
ASSERT(dst_type.IsZoneHandle() || dst_type.IsReadOnlyHandle());
DEBUG_ASSERT(dst_type.IsNotTemporaryScopedHandle());
ASSERT(!dst_type.IsNull());
static_calls_target_table_.Add(new (zone()) StaticCallsStruct(
Code::kPcRelativeTTSCall, Code::kDefaultEntry, assembler()->CodeSize(),
Expand All @@ -967,7 +967,7 @@ void FlowGraphCompiler::AddPcRelativeTTSCallTypeTarget(

void FlowGraphCompiler::AddStaticCallTarget(const Function& func,
Code::EntryKind entry_kind) {
ASSERT(func.IsZoneHandle());
DEBUG_ASSERT(func.IsNotTemporaryScopedHandle());
const auto entry_point = entry_kind == Code::EntryKind::kUnchecked
? Code::kUncheckedEntry
: Code::kDefaultEntry;
Expand All @@ -977,7 +977,7 @@ void FlowGraphCompiler::AddStaticCallTarget(const Function& func,
}

void FlowGraphCompiler::AddStubCallTarget(const Code& code) {
ASSERT(code.IsZoneHandle() || code.IsReadOnlyHandle());
DEBUG_ASSERT(code.IsNotTemporaryScopedHandle());
static_calls_target_table_.Add(new (zone()) StaticCallsStruct(
Code::kCallViaCode, Code::kDefaultEntry, assembler()->CodeSize(), nullptr,
&code, nullptr));
Expand Down
10 changes: 5 additions & 5 deletions runtime/vm/compiler/backend/flow_graph_compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -1218,11 +1218,11 @@ class FlowGraphCompiler : public ValueObject {
function(function_arg),
code(code_arg),
dst_type(dst_type) {
ASSERT(function == nullptr || function->IsZoneHandle());
ASSERT(code == nullptr || code->IsZoneHandle() ||
code->IsReadOnlyHandle());
ASSERT(dst_type == nullptr || dst_type->IsZoneHandle() ||
dst_type->IsReadOnlyHandle());
DEBUG_ASSERT(function == nullptr ||
function->IsNotTemporaryScopedHandle());
DEBUG_ASSERT(code == nullptr || code->IsNotTemporaryScopedHandle());
DEBUG_ASSERT(dst_type == nullptr ||
dst_type->IsNotTemporaryScopedHandle());
ASSERT(code == nullptr || dst_type == nullptr);
}

Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/compiler/backend/flow_graph_compiler_arm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ void FlowGraphCompiler::GenerateMethodExtractorIntrinsic(
intptr_t type_arguments_field_offset) {
// No frame has been setup here.
ASSERT(!__ constant_pool_allowed());
ASSERT(extracted_method.IsZoneHandle());
DEBUG_ASSERT(extracted_method.IsNotTemporaryScopedHandle());

const Code& build_method_extractor =
Code::ZoneHandle(extracted_method.IsGeneric()
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ void FlowGraphCompiler::GenerateMethodExtractorIntrinsic(
intptr_t type_arguments_field_offset) {
// No frame has been setup here.
ASSERT(!__ constant_pool_allowed());
ASSERT(extracted_method.IsZoneHandle());
DEBUG_ASSERT(extracted_method.IsNotTemporaryScopedHandle());

const Code& build_method_extractor =
Code::ZoneHandle(extracted_method.IsGeneric()
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/compiler/backend/flow_graph_compiler_riscv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ void FlowGraphCompiler::GenerateMethodExtractorIntrinsic(
intptr_t type_arguments_field_offset) {
// No frame has been setup here.
ASSERT(!__ constant_pool_allowed());
ASSERT(extracted_method.IsZoneHandle());
DEBUG_ASSERT(extracted_method.IsNotTemporaryScopedHandle());

const Code& build_method_extractor =
Code::ZoneHandle(extracted_method.IsGeneric()
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/compiler/backend/flow_graph_compiler_x64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ void FlowGraphCompiler::GenerateMethodExtractorIntrinsic(
intptr_t type_arguments_field_offset) {
// No frame has been setup here.
ASSERT(!__ constant_pool_allowed());
ASSERT(extracted_method.IsZoneHandle());
DEBUG_ASSERT(extracted_method.IsNotTemporaryScopedHandle());

const Code& build_method_extractor =
Code::ZoneHandle(extracted_method.IsGeneric()
Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/compiler/backend/il.cc
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ intptr_t Boxing::BoxCid(Representation rep) {

#if defined(DEBUG)
void Instruction::CheckField(const Field& field) const {
ASSERT(field.IsZoneHandle());
DEBUG_ASSERT(field.IsNotTemporaryScopedHandle());
ASSERT(!Compiler::IsBackgroundCompilation() || !field.IsOriginal());
}
#endif // DEBUG
Expand Down Expand Up @@ -5208,13 +5208,13 @@ bool CallTargets::HasSingleTarget() const {

const Function& CallTargets::FirstTarget() const {
ASSERT(length() != 0);
ASSERT(TargetAt(0)->target->IsZoneHandle());
DEBUG_ASSERT(TargetAt(0)->target->IsNotTemporaryScopedHandle());
return *TargetAt(0)->target;
}

const Function& CallTargets::MostPopularTarget() const {
ASSERT(length() != 0);
ASSERT(TargetAt(0)->target->IsZoneHandle());
DEBUG_ASSERT(TargetAt(0)->target->IsNotTemporaryScopedHandle());
for (int i = 1; i < length(); i++) {
ASSERT(TargetAt(i)->count <= TargetAt(0)->count);
}
Expand Down
33 changes: 16 additions & 17 deletions runtime/vm/compiler/backend/il.h
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ struct TargetInfo : public CidRange {
target(target_arg),
count(count_arg),
exactness(exactness) {
ASSERT(target->IsZoneHandle());
DEBUG_ASSERT(target->IsNotTemporaryScopedHandle());
}
const Function* target;
intptr_t count;
Expand Down Expand Up @@ -4275,7 +4275,7 @@ class TemplateDartCall : public VariadicDefinition {
type_args_len_(type_args_len),
argument_names_(argument_names),
token_pos_(source.token_pos) {
ASSERT(argument_names.IsZoneHandle() || argument_names.InVMIsolateHeap());
DEBUG_ASSERT(argument_names.IsNotTemporaryScopedHandle());
ASSERT(InputCount() >= kExtraInputs);
}

Expand Down Expand Up @@ -4412,9 +4412,9 @@ class InstanceCallBaseInstr : public TemplateDartCall<0> {
entry_kind_(Code::EntryKind::kNormal),
receiver_is_not_smi_(false),
is_call_on_this_(false) {
ASSERT(function_name.IsNotTemporaryScopedHandle());
ASSERT(interface_target.IsNotTemporaryScopedHandle());
ASSERT(tearoff_interface_target.IsNotTemporaryScopedHandle());
DEBUG_ASSERT(function_name.IsNotTemporaryScopedHandle());
DEBUG_ASSERT(interface_target.IsNotTemporaryScopedHandle());
DEBUG_ASSERT(tearoff_interface_target.IsNotTemporaryScopedHandle());
ASSERT(InputCount() > 0);
ASSERT(Token::IsBinaryOperator(token_kind) ||
Token::IsEqualityOperator(token_kind) ||
Expand Down Expand Up @@ -4750,7 +4750,7 @@ class DispatchTableCallInstr : public TemplateDartCall<1> {
interface_target_(interface_target),
selector_(selector) {
ASSERT(selector != nullptr);
ASSERT(interface_target_.IsNotTemporaryScopedHandle());
DEBUG_ASSERT(interface_target_.IsNotTemporaryScopedHandle());
ASSERT(InputCount() > 0);
}

Expand Down Expand Up @@ -5182,7 +5182,7 @@ class StaticCallInstr : public TemplateDartCall<0> {
is_known_list_constructor_(false),
entry_kind_(Code::EntryKind::kNormal),
identity_(AliasIdentity::Unknown()) {
ASSERT(function.IsZoneHandle());
DEBUG_ASSERT(function.IsNotTemporaryScopedHandle());
ASSERT(!function.IsNull());
}

Expand All @@ -5207,7 +5207,7 @@ class StaticCallInstr : public TemplateDartCall<0> {
is_known_list_constructor_(false),
entry_kind_(Code::EntryKind::kNormal),
identity_(AliasIdentity::Unknown()) {
ASSERT(function.IsZoneHandle());
DEBUG_ASSERT(function.IsNotTemporaryScopedHandle());
ASSERT(!function.IsNull());
}

Expand Down Expand Up @@ -5560,8 +5560,8 @@ class NativeCallInstr : public TemplateDartCall<0> {
function_(function),
token_pos_(source.token_pos),
link_lazily_(link_lazily) {
ASSERT(name.IsZoneHandle());
ASSERT(function.IsZoneHandle());
DEBUG_ASSERT(name.IsNotTemporaryScopedHandle());
DEBUG_ASSERT(function.IsNotTemporaryScopedHandle());
}

DECLARE_INSTRUCTION(NativeCall)
Expand Down Expand Up @@ -6184,7 +6184,7 @@ class StoreStaticFieldInstr : public TemplateDefinition<1, NoThrow> {
: TemplateDefinition(source),
field_(field),
token_pos_(source.token_pos) {
ASSERT(field.IsZoneHandle());
DEBUG_ASSERT(field.IsNotTemporaryScopedHandle());
SetInputAt(kValuePos, value);
CheckField(field);
}
Expand Down Expand Up @@ -6828,7 +6828,7 @@ class AllocateObjectInstr : public AllocationInstr {
has_type_arguments_(type_arguments != nullptr),
type_arguments_slot_(nullptr),
type_arguments_(type_arguments) {
ASSERT(cls.IsZoneHandle());
DEBUG_ASSERT(cls.IsNotTemporaryScopedHandle());
ASSERT(!cls.IsNull());
ASSERT((cls.NumTypeArguments() > 0) == has_type_arguments_);
if (has_type_arguments_) {
Expand Down Expand Up @@ -7492,7 +7492,7 @@ class InstantiateTypeInstr : public TemplateDefinition<2, Throws> {
: TemplateDefinition(source, deopt_id),
token_pos_(source.token_pos),
type_(type) {
ASSERT(type.IsZoneHandle() || type.IsReadOnlyHandle());
DEBUG_ASSERT(type.IsNotTemporaryScopedHandle());
SetInputAt(0, instantiator_type_arguments);
SetInputAt(1, function_type_arguments);
}
Expand Down Expand Up @@ -7542,9 +7542,8 @@ class InstantiateTypeArgumentsInstr : public TemplateDefinition<3, Throws> {
token_pos_(source.token_pos),
instantiator_class_(instantiator_class),
function_(function) {
ASSERT(instantiator_class.IsReadOnlyHandle() ||
instantiator_class.IsZoneHandle());
ASSERT(function.IsReadOnlyHandle() || function.IsZoneHandle());
DEBUG_ASSERT(instantiator_class.IsNotTemporaryScopedHandle());
DEBUG_ASSERT(function.IsNotTemporaryScopedHandle());
SetInputAt(0, instantiator_type_arguments);
SetInputAt(1, function_type_arguments);
SetInputAt(2, type_arguments);
Expand Down Expand Up @@ -9806,7 +9805,7 @@ class CheckNullInstr : public TemplateDefinition<1, Throws, Pure> {
token_pos_(source.token_pos),
function_name_(function_name),
exception_type_(exception_type) {
ASSERT(function_name.IsNotTemporaryScopedHandle());
DEBUG_ASSERT(function_name.IsNotTemporaryScopedHandle());
ASSERT(function_name.IsSymbol());
SetInputAt(0, value);
}
Expand Down
Loading

0 comments on commit 715866a

Please sign in to comment.