From dd3ec65af82a98bd1931b91c8a9340f28ba8490e Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Mon, 14 May 2018 20:11:27 +0000 Subject: [PATCH] [vm] Hash canonical objects by content instead of address. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: https://github.com/dart-lang/sdk/issues/31427 Change-Id: Iad02b152887576f8fbdcbed6892464a0c2ffa09c Reviewed-on: https://dart-review.googlesource.com/54662 Commit-Queue: Ryan Macnak Reviewed-by: Régis Crelier --- runtime/vm/clustered_snapshot.cc | 16 ------------ runtime/vm/isolate_reload.cc | 4 +-- runtime/vm/object.cc | 44 +++++++++++++++++++------------- runtime/vm/object.h | 38 +++++++++++++-------------- runtime/vm/pages.cc | 8 ------ runtime/vm/program_visitor.cc | 4 +-- 6 files changed, 47 insertions(+), 67 deletions(-) diff --git a/runtime/vm/clustered_snapshot.cc b/runtime/vm/clustered_snapshot.cc index e4652225541..e23953ecb74 100644 --- a/runtime/vm/clustered_snapshot.cc +++ b/runtime/vm/clustered_snapshot.cc @@ -256,22 +256,6 @@ class ClassDeserializationCluster : public DeserializationCluster { } } - void PostLoad(const Array& refs, Snapshot::Kind kind, Zone* zone) { - NOT_IN_PRODUCT(TimelineDurationScope tds( - Thread::Current(), Timeline::GetIsolateStream(), "PostLoadClass")); - - Class& cls = Class::Handle(zone); - for (intptr_t i = predefined_start_index_; i < predefined_stop_index_; - i++) { - cls ^= refs.At(i); - cls.RehashConstants(zone); - } - for (intptr_t i = start_index_; i < stop_index_; i++) { - cls ^= refs.At(i); - cls.RehashConstants(zone); - } - } - private: intptr_t predefined_start_index_; intptr_t predefined_stop_index_; diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc index 984de86f595..d28297aee9a 100644 --- a/runtime/vm/isolate_reload.cc +++ b/runtime/vm/isolate_reload.cc @@ -1501,8 +1501,8 @@ void IsolateReloadContext::Commit() { Become::ElementsForwardIdentity(before, after); } - // Rehash constants map for all classes. Constants are hashed by address, and - // addresses may change during a become operation. + // Rehash constants map for all classes. Constants are hashed by content, and + // content may have changed from fields being added or removed. { TIMELINE_SCOPE(RehashConstants); I->RehashConstants(); diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 0aff5d916c6..63d0a8202ce 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -4637,7 +4637,7 @@ class CanonicalInstanceKey { } return false; } - uword Hash() const { return key_.ComputeCanonicalTableHash(); } + uword Hash() const { return key_.CanonicalizeHash(); } const Instance& key_; private: @@ -4662,7 +4662,7 @@ class CanonicalInstanceTraits { static uword Hash(const Object& key) { ASSERT(!(key.IsString() || key.IsNumber() || key.IsAbstractType())); ASSERT(key.IsInstance()); - return Instance::Cast(key).ComputeCanonicalTableHash(); + return Instance::Cast(key).CanonicalizeHash(); } static uword Hash(const CanonicalInstanceKey& key) { return key.Hash(); } static RawObject* NewKey(const CanonicalInstanceKey& obj) { @@ -15670,20 +15670,22 @@ bool Instance::CanonicalizeEquals(const Instance& other) const { return true; } -uword Instance::ComputeCanonicalTableHash() const { - ASSERT(!IsNull()); +uint32_t Instance::CanonicalizeHash() const { + if (IsNull()) { + return 2011; + } NoSafepointScope no_safepoint; const intptr_t instance_size = SizeFromClass(); ASSERT(instance_size != 0); - uword hash = instance_size; + uint32_t hash = instance_size; uword this_addr = reinterpret_cast(this->raw_ptr()); + Instance& member = Instance::Handle(); for (intptr_t offset = Instance::NextFieldOffset(); offset < instance_size; offset += kWordSize) { - uword value = reinterpret_cast( - *reinterpret_cast(this_addr + offset)); - hash = CombineHashes(hash, value); + member ^= *reinterpret_cast(this_addr + offset); + hash = CombineHashes(hash, member.CanonicalizeHash()); } - return FinalizeHash(hash, (kBitsPerWord - 1)); + return FinalizeHash(hash, String::kHashBits); } #if defined(DEBUG) @@ -19383,6 +19385,10 @@ bool Double::CanonicalizeEquals(const Instance& other) const { return BitwiseEqualsToDouble(Double::Cast(other).value()); } +uint32_t Double::CanonicalizeHash() const { + return Hash64To32(bit_cast(value())); +} + RawDouble* Double::New(double d, Heap::Space space) { ASSERT(Isolate::Current()->object_store()->double_class() != Class::null()); Double& result = Double::Handle(); @@ -21784,16 +21790,18 @@ bool Array::CanonicalizeEquals(const Instance& other) const { return true; } -uword Array::ComputeCanonicalTableHash() const { - ASSERT(!IsNull()); +uint32_t Array::CanonicalizeHash() const { NoSafepointScope no_safepoint; intptr_t len = Length(); - uword hash = len; - uword value = reinterpret_cast(GetTypeArguments()); - hash = CombineHashes(hash, value); + if (len == 0) { + return 1; + } + uint32_t hash = len; + Instance& member = Instance::Handle(GetTypeArguments()); + hash = CombineHashes(hash, member.CanonicalizeHash()); for (intptr_t i = 0; i < len; i++) { - value = reinterpret_cast(At(i)); - hash = CombineHashes(hash, value); + member ^= At(i); + hash = CombineHashes(hash, member.CanonicalizeHash()); } return FinalizeHash(hash, kHashBits); } @@ -22410,12 +22418,12 @@ bool TypedData::CanonicalizeEquals(const Instance& other) const { (memcmp(DataAddr(0), other_typed_data.DataAddr(0), len) == 0); } -uword TypedData::ComputeCanonicalTableHash() const { +uint32_t TypedData::CanonicalizeHash() const { const intptr_t len = this->LengthInBytes(); if (len == 0) { return 1; } - uword hash = len; + uint32_t hash = len; for (intptr_t i = 0; i < len; i++) { hash = CombineHashes(len, GetUint8(i)); } diff --git a/runtime/vm/object.h b/runtime/vm/object.h index a422fe07304..c439f0dfdbb 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -5487,7 +5487,7 @@ class Instance : public Object { virtual bool OperatorEquals(const Instance& other) const; bool IsIdenticalTo(const Instance& other) const; virtual bool CanonicalizeEquals(const Instance& other) const; - virtual uword ComputeCanonicalTableHash() const; + virtual uint32_t CanonicalizeHash() const; intptr_t SizeFromClass() const { #if defined(DEBUG) @@ -5878,6 +5878,10 @@ class TypeArguments : public Instance { (len * kBytesPerElement)); } + virtual uint32_t CanonicalizeHash() const { + // Hash() is not stable until finalization is done. + return 0; + } intptr_t Hash() const; static RawTypeArguments* New(intptr_t len, Heap::Space space = Heap::kOld); @@ -5957,6 +5961,7 @@ class AbstractType : public Instance { virtual bool CanonicalizeEquals(const Instance& other) const { return Equals(other); } + virtual uint32_t CanonicalizeHash() const { return Hash(); } virtual bool Equals(const Instance& other) const { return IsEquivalent(other); } @@ -6733,10 +6738,7 @@ class Integer : public Number { virtual bool CanonicalizeEquals(const Instance& other) const { return Equals(other); } - virtual uword ComputeCanonicalTableHash() const { - UNREACHABLE(); - return 0; - } + virtual uint32_t CanonicalizeHash() const { return AsTruncatedUint32Value(); } virtual bool Equals(const Instance& other) const; virtual RawObject* HashCode() const { return raw(); } @@ -7013,10 +7015,7 @@ class Double : public Number { bool BitwiseEqualsToDouble(double value) const; virtual bool OperatorEquals(const Instance& other) const; virtual bool CanonicalizeEquals(const Instance& other) const; - virtual uword ComputeCanonicalTableHash() const { - UNREACHABLE(); - return 0; - } + virtual uint32_t CanonicalizeHash() const; static RawDouble* New(double d, Heap::Space space = Heap::kNew); @@ -7166,10 +7165,7 @@ class String : public Instance { virtual bool CanonicalizeEquals(const Instance& other) const { return Equals(other); } - virtual uword ComputeCanonicalTableHash() const { - UNREACHABLE(); - return 0; - } + virtual uint32_t CanonicalizeHash() const { return Hash(); } virtual bool Equals(const Instance& other) const; intptr_t CompareTo(const String& other) const; @@ -7844,6 +7840,10 @@ class Bool : public Instance { return value ? Bool::True() : Bool::False(); } + virtual uint32_t CanonicalizeHash() const { + return raw() == True().raw() ? 1231 : 1237; + } + private: void set_value(bool value) const { StoreNonPointer(&raw_ptr()->value_, value); @@ -7900,7 +7900,7 @@ class Array : public Instance { } virtual bool CanonicalizeEquals(const Instance& other) const; - virtual uword ComputeCanonicalTableHash() const; + virtual uint32_t CanonicalizeHash() const; static const intptr_t kBytesPerElement = kWordSize; static const intptr_t kMaxElements = kSmiMax / kBytesPerElement; @@ -8074,10 +8074,6 @@ class GrowableObjectArray : public Instance { UNREACHABLE(); return false; } - virtual uword ComputeCanonicalTableHash() const { - UNREACHABLE(); - return 0; - } // We don't expect a growable object array to be canonicalized. virtual RawInstance* CheckAndCanonicalize(Thread* thread, @@ -8253,7 +8249,7 @@ class TypedData : public Instance { } virtual bool CanonicalizeEquals(const Instance& other) const; - virtual uword ComputeCanonicalTableHash() const; + virtual uint32_t CanonicalizeHash() const; #define TYPED_GETTER_SETTER(name, type) \ type Get##name(intptr_t byte_offset) const { \ @@ -8778,7 +8774,9 @@ class Closure : public Instance { // None of the fields of a closure are instances. return true; } - + virtual uint32_t CanonicalizeHash() const { + return Function::Handle(function()).Hash(); + } int64_t ComputeHash() const; static RawClosure* New(const TypeArguments& instantiator_type_arguments, diff --git a/runtime/vm/pages.cc b/runtime/vm/pages.cc index ff43451c036..df7e04a8155 100644 --- a/runtime/vm/pages.cc +++ b/runtime/vm/pages.cc @@ -1075,14 +1075,6 @@ void PageSpace::CollectGarbage(bool compact) { set_tasks(tasks() - 1); ml.NotifyAll(); } - - if (compact) { - // Const object tables are hashed by address: rehash. - SafepointOperationScope safepoint(thread); - StackZone zone(thread); - HANDLESCOPE(thread); - thread->isolate()->RehashConstants(); - } } void PageSpace::BlockingSweep() { diff --git a/runtime/vm/program_visitor.cc b/runtime/vm/program_visitor.cc index 14148be2b0e..a6400e89088 100644 --- a/runtime/vm/program_visitor.cc +++ b/runtime/vm/program_visitor.cc @@ -311,9 +311,7 @@ class TypedDataKeyValueTrait { static Value ValueOf(Pair kv) { return kv; } - static inline intptr_t Hashcode(Key key) { - return key->ComputeCanonicalTableHash(); - } + static inline intptr_t Hashcode(Key key) { return key->CanonicalizeHash(); } static inline bool IsKeyEqual(Pair pair, Key key) { return pair->CanonicalizeEquals(*key);