Revision: 21616
Author: [email protected]
Date: Tue Jun 3 00:53:16 2014 UTC
Log: Split Put into Put and Remove
No longer treat the hole as a removal. This removes one branch in
Put and cleans up the API.
BUG=None
LOG=Y
[email protected], [email protected]
Review URL: https://codereview.chromium.org/309663005
http://code.google.com/p/v8/source/detail?r=21616
Modified:
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/test/cctest/test-dictionary.cc
/branches/bleeding_edge/test/cctest/test-ordered-hash-table.cc
=======================================
--- /branches/bleeding_edge/src/objects.cc Tue Jun 3 00:34:01 2014 UTC
+++ /branches/bleeding_edge/src/objects.cc Tue Jun 3 00:53:16 2014 UTC
@@ -5214,7 +5214,8 @@
if (inline_value->IsUndefined() || inline_value->IsSmi()) return;
Handle<ObjectHashTable> hashtable(ObjectHashTable::cast(inline_value));
- ObjectHashTable::Put(hashtable, key,
isolate->factory()->the_hole_value());
+ bool was_present = false;
+ ObjectHashTable::Remove(hashtable, key, &was_present);
}
@@ -16039,6 +16040,7 @@
Handle<Object> key,
Handle<Object> value) {
ASSERT(table->IsKey(*key));
+ ASSERT(!value->IsTheHole());
Isolate* isolate = table->GetIsolate();
@@ -16046,13 +16048,6 @@
Handle<Smi> hash = Object::GetOrCreateHash(isolate, key);
int entry = table->FindEntry(key);
-
- // Check whether to perform removal operation.
- if (value->IsTheHole()) {
- if (entry == kNotFound) return table;
- table->RemoveEntry(entry);
- return Shrink(table, key);
- }
// Key is already in table, just overwrite value.
if (entry != kNotFound) {
@@ -16067,6 +16062,29 @@
*value);
return table;
}
+
+
+Handle<ObjectHashTable> ObjectHashTable::Remove(Handle<ObjectHashTable>
table,
+ Handle<Object> key,
+ bool* was_present) {
+ ASSERT(table->IsKey(*key));
+
+ Object* hash = key->GetHash();
+ if (hash->IsUndefined()) {
+ *was_present = false;
+ return table;
+ }
+
+ int entry = table->FindEntry(key);
+ if (entry == kNotFound) {
+ *was_present = false;
+ return table;
+ }
+
+ *was_present = true;
+ table->RemoveEntry(entry);
+ return Shrink(table, key);
+}
void ObjectHashTable::AddEntry(int entry, Object* key, Object* value) {
@@ -16198,6 +16216,20 @@
return new_table;
}
+
+
+template<class Derived, class Iterator, int entrysize>
+Handle<Derived> OrderedHashTable<Derived, Iterator, entrysize>::Remove(
+ Handle<Derived> table, Handle<Object> key, bool* was_present) {
+ int entry = table->FindEntry(key);
+ if (entry == kNotFound) {
+ *was_present = false;
+ return table;
+ }
+ *was_present = true;
+ table->RemoveEntry(entry);
+ return Shrink(table);
+}
template<class Derived, class Iterator, int entrysize>
@@ -16309,6 +16341,10 @@
OrderedHashTable<OrderedHashSet, JSSetIterator, 1>::Clear(
Handle<OrderedHashSet> table);
+template Handle<OrderedHashSet>
+OrderedHashTable<OrderedHashSet, JSSetIterator, 1>::Remove(
+ Handle<OrderedHashSet> table, Handle<Object> key, bool* was_present);
+
template int
OrderedHashTable<OrderedHashSet, JSSetIterator, 1>::FindEntry(
Handle<Object> key);
@@ -16336,6 +16372,10 @@
OrderedHashTable<OrderedHashMap, JSMapIterator, 2>::Clear(
Handle<OrderedHashMap> table);
+template Handle<OrderedHashMap>
+OrderedHashTable<OrderedHashMap, JSMapIterator, 2>::Remove(
+ Handle<OrderedHashMap> table, Handle<Object> key, bool* was_present);
+
template int
OrderedHashTable<OrderedHashMap, JSMapIterator, 2>::FindEntry(
Handle<Object> key);
@@ -16363,20 +16403,6 @@
table->set(index, *key);
return table;
}
-
-
-Handle<OrderedHashSet> OrderedHashSet::Remove(Handle<OrderedHashSet> table,
- Handle<Object> key,
- bool* was_present) {
- int entry = table->FindEntry(key);
- if (entry == kNotFound) {
- *was_present = false;
- return table;
- }
- *was_present = true;
- table->RemoveEntry(entry);
- return Shrink(table);
-}
Object* OrderedHashMap::Lookup(Handle<Object> key) {
@@ -16390,14 +16416,10 @@
Handle<OrderedHashMap> OrderedHashMap::Put(Handle<OrderedHashMap> table,
Handle<Object> key,
Handle<Object> value) {
+ ASSERT(!key->IsTheHole());
+
int entry = table->FindEntry(key);
- if (value->IsTheHole()) {
- if (entry == kNotFound) return table;
- table->RemoveEntry(entry);
- return Shrink(table);
- }
-
if (entry != kNotFound) {
table->set(table->EntryToIndex(entry) + kValueOffset, *value);
return table;
=======================================
--- /branches/bleeding_edge/src/objects.h Tue Jun 3 00:34:01 2014 UTC
+++ /branches/bleeding_edge/src/objects.h Tue Jun 3 00:53:16 2014 UTC
@@ -4133,12 +4133,16 @@
// returned in case the key is not present.
Object* Lookup(Handle<Object> key);
- // Adds (or overwrites) the value associated with the given key. Mapping
a
- // key to the hole value causes removal of the whole entry.
+ // Adds (or overwrites) the value associated with the given key.
static Handle<ObjectHashTable> Put(Handle<ObjectHashTable> table,
Handle<Object> key,
Handle<Object> value);
+ // Returns an ObjectHashTable (possibly |table|) where |key| has been
removed.
+ static Handle<ObjectHashTable> Remove(Handle<ObjectHashTable> table,
+ Handle<Object> key,
+ bool* was_present);
+
private:
friend class MarkCompactCollector;
@@ -4203,10 +4207,15 @@
// if possible.
static Handle<Derived> Shrink(Handle<Derived> table);
- // Returns a new empty OrderedHashTable and updates all the iterators to
- // point to the new table.
+ // Returns a new empty OrderedHashTable and records the clearing so that
+ // exisiting iterators can be updated.
static Handle<Derived> Clear(Handle<Derived> table);
+ // Returns an OrderedHashTable (possibly |table|) where |key| has been
+ // removed.
+ static Handle<Derived> Remove(Handle<Derived> table, Handle<Object> key,
+ bool* was_present);
+
// Returns kNotFound if the key isn't present.
int FindEntry(Handle<Object> key);
@@ -4330,8 +4339,6 @@
bool Contains(Handle<Object> key);
static Handle<OrderedHashSet> Add(
Handle<OrderedHashSet> table, Handle<Object> key);
- static Handle<OrderedHashSet> Remove(
- Handle<OrderedHashSet> table, Handle<Object> key, bool* was_present);
};
=======================================
--- /branches/bleeding_edge/src/runtime.cc Tue Jun 3 00:34:01 2014 UTC
+++ /branches/bleeding_edge/src/runtime.cc Tue Jun 3 00:53:16 2014 UTC
@@ -1646,11 +1646,11 @@
CONVERT_ARG_HANDLE_CHECKED(JSMap, holder, 0);
CONVERT_ARG_HANDLE_CHECKED(Object, key, 1);
Handle<OrderedHashMap> table(OrderedHashMap::cast(holder->table()));
- Handle<Object> lookup(table->Lookup(key), isolate);
+ bool was_present = false;
Handle<OrderedHashMap> new_table =
- OrderedHashMap::Put(table, key,
isolate->factory()->the_hole_value());
+ OrderedHashMap::Remove(table, key, &was_present);
holder->set_table(*new_table);
- return isolate->heap()->ToBoolean(!lookup->IsTheHole());
+ return isolate->heap()->ToBoolean(was_present);
}
@@ -1767,11 +1767,11 @@
Handle<ObjectHashTable> table(ObjectHashTable::cast(
weak_collection->table()));
RUNTIME_ASSERT(table->IsKey(*key));
- Handle<Object> lookup(table->Lookup(key), isolate);
+ bool was_present = false;
Handle<ObjectHashTable> new_table =
- ObjectHashTable::Put(table, key,
isolate->factory()->the_hole_value());
+ ObjectHashTable::Remove(table, key, &was_present);
weak_collection->set_table(*new_table);
- return isolate->heap()->ToBoolean(!lookup->IsTheHole());
+ return isolate->heap()->ToBoolean(was_present);
}
=======================================
--- /branches/bleeding_edge/test/cctest/test-dictionary.cc Wed May 21
12:31:31 2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-dictionary.cc Tue Jun 3
00:53:16 2014 UTC
@@ -51,6 +51,7 @@
table = HashMap::Put(table, a, b);
CHECK_EQ(table->NumberOfElements(), 1);
CHECK_EQ(table->Lookup(a), *b);
+ // When the key does not exist in the map, Lookup returns the hole.
CHECK_EQ(table->Lookup(b), CcTest::heap()->the_hole_value());
// Keys still have to be valid after objects were moved.
@@ -64,8 +65,10 @@
CHECK_EQ(table->NumberOfElements(), 1);
CHECK_NE(table->Lookup(a), *b);
- // Keys mapped to the hole should be removed permanently.
- table = HashMap::Put(table, a, factory->the_hole_value());
+ // Keys that have been removed are mapped to the hole.
+ bool was_present = false;
+ table = HashMap::Remove(table, a, &was_present);
+ CHECK(was_present);
CHECK_EQ(table->NumberOfElements(), 0);
CHECK_EQ(table->Lookup(a), CcTest::heap()->the_hole_value());
=======================================
--- /branches/bleeding_edge/test/cctest/test-ordered-hash-table.cc Tue Jun
3 00:34:01 2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-ordered-hash-table.cc Tue Jun
3 00:53:16 2014 UTC
@@ -123,8 +123,9 @@
ordered_map = OrderedHashMap::Put(ordered_map, obj, val);
CHECK_EQ(1, ordered_map->NumberOfElements());
CHECK(ordered_map->Lookup(obj)->SameValue(*val));
- ordered_map = OrderedHashMap::Put(
- ordered_map, obj, factory->the_hole_value());
+ bool was_present = false;
+ ordered_map = OrderedHashMap::Remove(ordered_map, obj, &was_present);
+ CHECK(was_present);
CHECK_EQ(0, ordered_map->NumberOfElements());
CHECK(ordered_map->Lookup(obj)->IsTheHole());
@@ -157,14 +158,14 @@
CHECK_EQ(4, ordered_map->NumberOfBuckets());
// Test shrinking
- ordered_map = OrderedHashMap::Put(
- ordered_map, obj, factory->the_hole_value());
- ordered_map = OrderedHashMap::Put(
- ordered_map, obj1, factory->the_hole_value());
- ordered_map = OrderedHashMap::Put(
- ordered_map, obj2, factory->the_hole_value());
- ordered_map = OrderedHashMap::Put(
- ordered_map, obj3, factory->the_hole_value());
+ ordered_map = OrderedHashMap::Remove(ordered_map, obj, &was_present);
+ CHECK(was_present);
+ ordered_map = OrderedHashMap::Remove(ordered_map, obj1, &was_present);
+ CHECK(was_present);
+ ordered_map = OrderedHashMap::Remove(ordered_map, obj2, &was_present);
+ CHECK(was_present);
+ ordered_map = OrderedHashMap::Remove(ordered_map, obj3, &was_present);
+ CHECK(was_present);
CHECK_EQ(1, ordered_map->NumberOfElements());
CHECK_EQ(2, ordered_map->NumberOfBuckets());
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
