Skip to content

Commit

Permalink
[vm] Refactor generation of identity hashes.
Browse files Browse the repository at this point in the history 
Generating identity hashes from the runtime no longer calls into Dart. On 32-bit systems, generating identity hashes from Dart now does only one runtime transition.

TEST=ci
Bug: #47873
Change-Id: Ib21156cb05706f81744eb4e5ccb644f40aa84c96
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/222326
Reviewed-by: Alexander Markov <alexmarkov@google.com>
  • Loading branch information
@rmacnak-google
rmacnak-google committed Dec 13, 2021
1 parent 08e0a56 commit ecdf148
Show file tree
Hide file tree
Showing 30 changed files with 230 additions and 295 deletions.
10 changes: 5 additions & 5 deletions pkg/front_end/testcases/general/invalid_operator.dart.weak.expect
View file
Original file line number Diff line number Diff line change
Expand Up @@ -564,35 +564,35 @@ library;
// Try adding explicit types.
// operator ==<T>(a) => true;
// ^^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is one of the overridden members.
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is one of the overridden members.
// external bool operator ==(Object other);
// ^^
//
// pkg/front_end/testcases/general/invalid_operator.dart:6:12: Error: The method 'Operators1.==' has fewer positional arguments than those of overridden method 'Object.=='.
// operator ==() => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
// pkg/front_end/testcases/general/invalid_operator.dart:27:12: Error: The method 'Operators2.==' has more required arguments than those of overridden method 'Object.=='.
// operator ==(a, b) => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
// pkg/front_end/testcases/general/invalid_operator.dart:71:12: Error: The method 'Operators4.==' has fewer positional arguments than those of overridden method 'Object.=='.
// operator ==({a}) => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
// pkg/front_end/testcases/general/invalid_operator.dart:137:12: Error: Declared type variables of 'Operators7.==' doesn't match those on overridden method 'Object.=='.
// operator ==<T>(a) => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
Expand Down
10 changes: 5 additions & 5 deletions pkg/front_end/testcases/general/invalid_operator.dart.weak.modular.expect
View file
Original file line number Diff line number Diff line change
Expand Up @@ -564,35 +564,35 @@ library;
// Try adding explicit types.
// operator ==<T>(a) => true;
// ^^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is one of the overridden members.
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is one of the overridden members.
// external bool operator ==(Object other);
// ^^
//
// pkg/front_end/testcases/general/invalid_operator.dart:6:12: Error: The method 'Operators1.==' has fewer positional arguments than those of overridden method 'Object.=='.
// operator ==() => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
// pkg/front_end/testcases/general/invalid_operator.dart:27:12: Error: The method 'Operators2.==' has more required arguments than those of overridden method 'Object.=='.
// operator ==(a, b) => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
// pkg/front_end/testcases/general/invalid_operator.dart:71:12: Error: The method 'Operators4.==' has fewer positional arguments than those of overridden method 'Object.=='.
// operator ==({a}) => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
// pkg/front_end/testcases/general/invalid_operator.dart:137:12: Error: Declared type variables of 'Operators7.==' doesn't match those on overridden method 'Object.=='.
// operator ==<T>(a) => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
Expand Down
10 changes: 5 additions & 5 deletions pkg/front_end/testcases/general/invalid_operator.dart.weak.outline.expect
View file
Original file line number Diff line number Diff line change
Expand Up @@ -564,35 +564,35 @@ library;
// Try adding explicit types.
// operator ==<T>(a) => true;
// ^^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is one of the overridden members.
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is one of the overridden members.
// external bool operator ==(Object other);
// ^^
//
// pkg/front_end/testcases/general/invalid_operator.dart:6:12: Error: The method 'Operators1.==' has fewer positional arguments than those of overridden method 'Object.=='.
// operator ==() => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
// pkg/front_end/testcases/general/invalid_operator.dart:27:12: Error: The method 'Operators2.==' has more required arguments than those of overridden method 'Object.=='.
// operator ==(a, b) => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
// pkg/front_end/testcases/general/invalid_operator.dart:71:12: Error: The method 'Operators4.==' has fewer positional arguments than those of overridden method 'Object.=='.
// operator ==({a}) => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
// pkg/front_end/testcases/general/invalid_operator.dart:137:12: Error: Declared type variables of 'Operators7.==' doesn't match those on overridden method 'Object.=='.
// operator ==<T>(a) => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
Expand Down
2 changes: 1 addition & 1 deletion pkg/front_end/testcases/nnbd/issue42603.dart.strong.expect
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ library /*isNonNullableByDefault*/;
// pkg/front_end/testcases/nnbd/issue42603.dart:18:17: Error: The method 'E.==' has fewer positional arguments than those of overridden method 'Object.=='.
// bool operator ==() => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
Expand Down
2 changes: 1 addition & 1 deletion pkg/front_end/testcases/nnbd/issue42603.dart.weak.expect
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ library /*isNonNullableByDefault*/;
// pkg/front_end/testcases/nnbd/issue42603.dart:18:17: Error: The method 'E.==' has fewer positional arguments than those of overridden method 'Object.=='.
// bool operator ==() => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
Expand Down
2 changes: 1 addition & 1 deletion pkg/front_end/testcases/nnbd/issue42603.dart.weak.modular.expect
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ library /*isNonNullableByDefault*/;
// pkg/front_end/testcases/nnbd/issue42603.dart:18:17: Error: The method 'E.==' has fewer positional arguments than those of overridden method 'Object.=='.
// bool operator ==() => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
Expand Down
2 changes: 1 addition & 1 deletion pkg/front_end/testcases/nnbd/issue42603.dart.weak.outline.expect
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ library /*isNonNullableByDefault*/;
// pkg/front_end/testcases/nnbd/issue42603.dart:18:17: Error: The method 'E.==' has fewer positional arguments than those of overridden method 'Object.=='.
// bool operator ==() => true;
// ^
// sdk/lib/_internal/vm/lib/object_patch.dart:29:26: Context: This is the overridden method ('==').
// sdk/lib/_internal/vm/lib/object_patch.dart:21:26: Context: This is the overridden method ('==').
// external bool operator ==(Object other);
// ^
//
Expand Down
2 changes: 1 addition & 1 deletion ...vm/testcases/transformations/type_flow/transformer/enum_from_lib_used_as_type.dart.expect
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ class Class extends core::Object {
synthetic constructor •() → self::Class
: super core::Object::•()
;
[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:3305,getterSelectorId:3306] method method([@vm.inferred-type.metadata=dart.core::Null? (value: null)] self::Enum e) → core::int
[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:3303,getterSelectorId:3304] method method([@vm.inferred-type.metadata=dart.core::Null? (value: null)] self::Enum e) → core::int
return [@vm.inferred-type.metadata=!] e.{core::_Enum::index}{core::int};
}
2 changes: 1 addition & 1 deletion pkg/vm/testcases/transformations/type_flow/transformer/tree_shake_enum_from_lib.dart.expect
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ class ConstClass extends core::Object {
synthetic constructor •() → self::ConstClass
: super core::Object::•()
;
[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:3309,getterSelectorId:3310] method method([@vm.inferred-type.metadata=dart.core::Null? (value: null)] self::ConstEnum e) → core::int
[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:3307,getterSelectorId:3308] method method([@vm.inferred-type.metadata=dart.core::Null? (value: null)] self::ConstEnum e) → core::int
return [@vm.inferred-type.metadata=!] e.{core::_Enum::index}{core::int};
}
16 changes: 5 additions & 11 deletions runtime/lib/object.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,14 @@ DEFINE_NATIVE_ENTRY(Object_getHash, 0, 1) {
// Please note that no handle is created for the argument.
// This is safe since the argument is only used in a tail call.
// The performance benefit is more than 5% when using hashCode.
return Smi::New(GetHash(isolate, arguments->NativeArgAt(0)));
}
intptr_t hash = GetHash(isolate, arguments->NativeArgAt(0));
if (LIKELY(hash != 0)) {
return Smi::New(hash);
}

DEFINE_NATIVE_ENTRY(Object_setHashIfNotSetYet, 0, 2) {
GET_NON_NULL_NATIVE_ARGUMENT(Smi, hash, arguments->NativeArgAt(1));
#if defined(HASH_IN_OBJECT_HEADER)
return Smi::New(
Object::SetCachedHashIfNotSet(arguments->NativeArgAt(0), hash.Value()));
#else
const Instance& instance =
Instance::CheckedHandle(zone, arguments->NativeArgAt(0));
Heap* heap = thread->heap();
return Smi::New(heap->SetHashIfNotSet(instance.ptr(), hash.Value()));
#endif
return instance.IdentityHashCode(arguments->thread());
}

DEFINE_NATIVE_ENTRY(Object_toString, 0, 1) {
Expand Down
1 change: 0 additions & 1 deletion runtime/vm/bootstrap_natives.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ namespace dart {
V(DartAsync_fatal, 1) \
V(Object_equals, 2) \
V(Object_getHash, 1) \
V(Object_setHashIfNotSetYet, 2) \
V(Object_toString, 1) \
V(Object_runtimeType, 1) \
V(Object_haveSameRuntimeType, 2) \
Expand Down
5 changes: 0 additions & 5 deletions runtime/vm/compiler/asm_intrinsifier_arm.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1548,11 +1548,6 @@ void AsmIntrinsifier::Object_getHash(Assembler* assembler,
UNREACHABLE();
}

void AsmIntrinsifier::Object_setHashIfNotSetYet(Assembler* assembler,
Label* normal_ir_body) {
UNREACHABLE();
}

void AsmIntrinsifier::StringBaseCharAt(Assembler* assembler,
Label* normal_ir_body) {
Label try_two_byte_string;
Expand Down
32 changes: 24 additions & 8 deletions runtime/vm/compiler/asm_intrinsifier_arm64.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1578,21 +1578,37 @@ void AsmIntrinsifier::FunctionType_equality(Assembler* assembler,
__ Bind(normal_ir_body);
}

// Keep in sync with Instance::IdentityHashCode.
// Note int and double never reach here because they override _identityHashCode.
// Special cases are also not needed for null or bool because they were pre-set
// during VM isolate finalization.
void AsmIntrinsifier::Object_getHash(Assembler* assembler,
Label* normal_ir_body) {
__ ldr(R0, Address(SP, 0 * target::kWordSize));
__ ldr(R0, FieldAddress(R0, target::String::hash_offset(), kFourBytes),
Label not_yet_computed;
__ ldr(R0, Address(SP, 0 * target::kWordSize)); // Object.
__ ldr(R0,
FieldAddress(R0,
target::Object::tags_offset() +
target::UntaggedObject::kHashTagPos / kBitsPerByte,
kFourBytes),
kUnsignedFourBytes);
__ cbz(&not_yet_computed, R0);
__ SmiTag(R0);
__ ret();
}

void AsmIntrinsifier::Object_setHashIfNotSetYet(Assembler* assembler,
Label* normal_ir_body) {
__ ldp(/*Value=*/R1, /*Object=*/R0, Address(SP, 0, Address::PairOffset));
// R0: Untagged address of header word (ldxr/stxr do not support offsets).
__ Bind(&not_yet_computed);
__ LoadFromOffset(R1, THR, target::Thread::random_offset());
__ AndImmediate(R2, R1, 0xffffffff); // state_lo
__ LsrImmediate(R3, R1, 32); // state_hi
__ LoadImmediate(R1, 0xffffda61); // A
__ mul(R1, R1, R2);
__ add(R1, R1, Operand(R3)); // new_state = (A * state_lo) + state_hi
__ StoreToOffset(R1, THR, target::Thread::random_offset());
__ AndImmediate(R1, R1, 0x3fffffff);
__ cbz(&not_yet_computed, R1);

__ ldr(R0, Address(SP, 0 * target::kWordSize)); // Object.
__ sub(R0, R0, Operand(kHeapObjectTag));
__ SmiUntag(R1);
__ LslImmediate(R3, R1, target::UntaggedObject::kHashTagPos);

Label retry, already_set_in_r4;
Expand Down
5 changes: 0 additions & 5 deletions runtime/vm/compiler/asm_intrinsifier_ia32.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1555,11 +1555,6 @@ void AsmIntrinsifier::Object_getHash(Assembler* assembler,
UNREACHABLE();
}

void AsmIntrinsifier::Object_setHashIfNotSetYet(Assembler* assembler,
Label* normal_ir_body) {
UNREACHABLE();
}

void AsmIntrinsifier::StringBaseCharAt(Assembler* assembler,
Label* normal_ir_body) {
Label try_two_byte_string;
Expand Down
74 changes: 0 additions & 74 deletions runtime/vm/compiler/asm_intrinsifier_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,78 +10,4 @@

namespace dart {

static intptr_t GetHash(Isolate* isolate, const ObjectPtr obj) {
#if defined(HASH_IN_OBJECT_HEADER)
return Object::GetCachedHash(obj);
#else
Heap* heap = isolate->group()->heap();
ASSERT(obj->IsDartInstance());
return heap->GetHash(obj);
#endif
}

ISOLATE_UNIT_TEST_CASE(AsmIntrinsifier_SetHashIfNotSetYet) {
auto I = Isolate::Current();
const auto& corelib = Library::Handle(Library::CoreLibrary());
const auto& name = String::Handle(String::New("_setHashIfNotSetYet"));
const auto& symbol = String::Handle(Symbols::New(thread, name));

const auto& function =
Function::Handle(corelib.LookupFunctionAllowPrivate(symbol));
const auto& object_class =
Class::Handle(corelib.LookupClass(Symbols::Object()));

auto& smi0 = Smi::Handle(Smi::New(0));
auto& smi21 = Smi::Handle(Smi::New(21));
auto& smi42 = Smi::Handle(Smi::New(42));
const auto& obj = Object::Handle(Instance::New(object_class));
const auto& args = Array::Handle(Array::New(2));

const auto& args_descriptor_array =
Array::Handle(ArgumentsDescriptor::NewBoxed(0, 2, Array::empty_array()));

// Initialized to 0
EXPECT_EQ(smi0.ptr(), Smi::New(GetHash(I, obj.ptr())));

// Lazily set to 42 on first call.
args.SetAt(0, obj);
args.SetAt(1, smi42);
EXPECT_EQ(smi42.ptr(),
DartEntry::InvokeFunction(function, args, args_descriptor_array));
EXPECT_EQ(smi42.ptr(), Smi::New(GetHash(I, obj.ptr())));

// Stays at 42 on subsequent calls.
args.SetAt(0, obj);
args.SetAt(1, smi21);
EXPECT_EQ(smi42.ptr(),
DartEntry::InvokeFunction(function, args, args_descriptor_array));
EXPECT_EQ(smi42.ptr(), Smi::New(GetHash(I, obj.ptr())));

// We test setting the maximum value our core libraries would use when
// installing an identity hash code (see
// sdk/lib/_internal/vm/lib/object_patch.dart:Object._objectHashCode)
//
// This value is representable as a positive Smi on all architectures (even
// compressed pointers).
const auto& smiMax = Smi::Handle(Smi::New(0x40000000 - 1));
const auto& obj2 = Object::Handle(Instance::New(object_class));

// Initialized to 0
EXPECT_EQ(smi0.ptr(), Smi::New(GetHash(I, obj2.ptr())));

// Lazily set to smiMax first call.
args.SetAt(0, obj2);
args.SetAt(1, smiMax);
EXPECT_EQ(smiMax.ptr(),
DartEntry::InvokeFunction(function, args, args_descriptor_array));
EXPECT_EQ(smiMax.ptr(), Smi::New(GetHash(I, obj2.ptr())));

// Stays at smiMax on subsequent calls.
args.SetAt(0, obj2);
args.SetAt(1, smi21);
EXPECT_EQ(smiMax.ptr(),
DartEntry::InvokeFunction(function, args, args_descriptor_array));
EXPECT_EQ(smiMax.ptr(), Smi::New(GetHash(I, obj2.ptr())));
}

} // namespace dart

0 comments on commit ecdf148

Please sign in to comment.