diff --git a/runtime/vm/hash_map.h b/runtime/vm/hash_map.h index 4b032f57874..04bfdd1b155 100644 --- a/runtime/vm/hash_map.h +++ b/runtime/vm/hash_map.h @@ -39,6 +39,15 @@ class BaseDirectChainedHashMap : public B { void Insert(typename KeyValueTrait::Pair kv); bool Remove(typename KeyValueTrait::Key key); + // If a pair already exists in the map with an equal key, replace that pair + // with this one. Otherwise, insert the pair as a new entry. + // + // Note: Insert operates in constant time, while Update must walk the chained + // entries for a given hash value, checking keys for equality. However, if + // multiple value updates are needed for the same key, only using Update + // guarantees constant space usage whereas Insert does not. + void Update(typename KeyValueTrait::Pair kv); + typename KeyValueTrait::Value LookupValue( typename KeyValueTrait::Key key) const; @@ -314,6 +323,20 @@ void BaseDirectChainedHashMap::Insert( } } +template +void BaseDirectChainedHashMap::Update( + typename KeyValueTrait::Pair kv) { + const typename KeyValueTrait::Value kNoValue = + KeyValueTrait::ValueOf(typename KeyValueTrait::Pair()); + + ASSERT(KeyValueTrait::ValueOf(kv) != kNoValue); + if (auto const old_kv = Lookup(KeyValueTrait::KeyOf(kv))) { + *old_kv = kv; + } else { + Insert(kv); + } +} + template bool BaseDirectChainedHashMap::Remove( typename KeyValueTrait::Key key) { diff --git a/runtime/vm/hash_map_test.cc b/runtime/vm/hash_map_test.cc index b2d8c00999d..a040869d646 100644 --- a/runtime/vm/hash_map_test.cc +++ b/runtime/vm/hash_map_test.cc @@ -220,4 +220,84 @@ TEST_CASE(CStringMap) { free(str1); } +TEST_CASE(CStringMapUpdate) { + const char* const kConst1 = "test"; + const char* const kConst2 = "test 2"; + + char* str1 = OS::SCreate(nullptr, "%s", kConst1); + char* str2 = OS::SCreate(nullptr, "%s", kConst2); + char* str3 = OS::SCreate(nullptr, "%s", kConst1); + char* str4 = OS::SCreate(nullptr, "%s", kConst1); // Only used for lookup. + + // Make sure these strings are pointer-distinct, but C-string-equal. + EXPECT_NE(str1, str3); + EXPECT_NE(str1, str4); + EXPECT_NE(str3, str4); + EXPECT_STREQ(str1, str3); + EXPECT_STREQ(str1, str4); + + CStringKeyValueTrait::Pair p1 = {str1, 1}; + CStringKeyValueTrait::Pair p2 = {str2, 2}; + CStringKeyValueTrait::Pair p3 = {str3, 3}; + + CStringMap map; + EXPECT(map.IsEmpty()); + + map.Update(p1); + EXPECT_NOTNULL(map.Lookup(str1)); + EXPECT_EQ(p1.value, map.LookupValue(str1)); + EXPECT_NULLPTR(map.Lookup(str2)); + EXPECT_NOTNULL(map.Lookup(str3)); + EXPECT_EQ(p1.value, map.LookupValue(str3)); + EXPECT_NOTNULL(map.Lookup(str4)); + EXPECT_EQ(p1.value, map.LookupValue(str4)); + + map.Update(p2); + EXPECT_NOTNULL(map.Lookup(str1)); + EXPECT_EQ(p1.value, map.LookupValue(str1)); + EXPECT_NOTNULL(map.Lookup(str2)); + EXPECT_EQ(p2.value, map.LookupValue(str2)); + EXPECT_NOTNULL(map.Lookup(str3)); + EXPECT_EQ(p1.value, map.LookupValue(str3)); + EXPECT_NOTNULL(map.Lookup(str4)); + EXPECT_EQ(p1.value, map.LookupValue(str4)); + + // Check Lookup after Update. + map.Update(p3); + EXPECT_NOTNULL(map.Lookup(str1)); + EXPECT_EQ(p3.value, map.LookupValue(str1)); + EXPECT_NOTNULL(map.Lookup(str2)); + EXPECT_EQ(p2.value, map.LookupValue(str2)); + EXPECT_NOTNULL(map.Lookup(str3)); + EXPECT_EQ(p3.value, map.LookupValue(str3)); + EXPECT_NOTNULL(map.Lookup(str4)); + EXPECT_EQ(p3.value, map.LookupValue(str4)); + + // Check that single Remove after only Updates ensures Lookup fails after. + EXPECT(map.Remove(str3)); + EXPECT_NULLPTR(map.Lookup(str1)); + EXPECT_NOTNULL(map.Lookup(str2)); + EXPECT_EQ(p2.value, map.LookupValue(str2)); + EXPECT_NULLPTR(map.Lookup(str3)); + EXPECT_NULLPTR(map.Lookup(str4)); + + EXPECT(!map.Remove(str3)); + EXPECT(map.Remove(str2)); + EXPECT(map.IsEmpty()); + + // Quick double-check that these weren't side-effected by the implementation + // of hash maps (p1 especially). + EXPECT_EQ(str1, p1.key); + EXPECT_EQ(1, p1.value); + EXPECT_EQ(str2, p2.key); + EXPECT_EQ(2, p2.value); + EXPECT_EQ(str3, p3.key); + EXPECT_EQ(3, p3.value); + + free(str4); + free(str3); + free(str2); + free(str1); +} + } // namespace dart