Reviewers: rossberg,
Message:
PTAL.
Description:
Fix identity hash code function to respect flag.
The flag passed to JSObject::GetIdentityHash() was not respected so far
and an indentity hash code was generated even when the flag requested
not to do so. This could lead to a rare corner cases (for which a test
case was added) where a GC request would have been dropped.
[email protected]
TEST=cctest/test-dictionary
Please review this at http://codereview.chromium.org/8390047/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/objects.cc
M test/cctest/test-dictionary.cc
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
8562c7094f98af7f1468d45edd45b005912bd6a3..9a87ac57d6552d01df1d994fe33133cbbffd4288
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -3589,6 +3589,9 @@ MaybeObject* JSObject::GetIdentityHash(CreationFlag
flag) {
Object* stored_value =
GetHiddenProperty(GetHeap()->identity_hash_symbol());
if (stored_value->IsSmi()) return stored_value;
+ // Do not generate permanent identity hash code if not requested.
+ if (flag == OMIT_CREATION) return GetHeap()->undefined_value();
+
Smi* hash = GenerateIdentityHash();
MaybeObject* result =
SetHiddenProperty(GetHeap()->identity_hash_symbol(),
hash);
@@ -12390,7 +12393,7 @@ MaybeObject*
StringDictionary::TransformPropertiesToFastFor(
bool ObjectHashSet::Contains(Object* key) {
// If the object does not have an identity hash, it was never used as a
key.
{ MaybeObject* maybe_hash = key->GetHash(OMIT_CREATION);
- if (maybe_hash->IsFailure()) return false;
+ if (maybe_hash->ToObjectUnchecked()->IsUndefined()) return false;
}
return (FindEntry(key) != kNotFound);
}
@@ -12424,7 +12427,7 @@ MaybeObject* ObjectHashSet::Add(Object* key) {
MaybeObject* ObjectHashSet::Remove(Object* key) {
// If the object does not have an identity hash, it was never used as a
key.
{ MaybeObject* maybe_hash = key->GetHash(OMIT_CREATION);
- if (maybe_hash->IsFailure()) return this;
+ if (maybe_hash->ToObjectUnchecked()->IsUndefined()) return this;
}
int entry = FindEntry(key);
@@ -12441,7 +12444,9 @@ MaybeObject* ObjectHashSet::Remove(Object* key) {
Object* ObjectHashTable::Lookup(Object* key) {
// If the object does not have an identity hash, it was never used as a
key.
{ MaybeObject* maybe_hash = key->GetHash(OMIT_CREATION);
- if (maybe_hash->IsFailure()) GetHeap()->undefined_value();
+ if (maybe_hash->ToObjectUnchecked()->IsUndefined()) {
+ return GetHeap()->undefined_value();
+ }
}
int entry = FindEntry(key);
if (entry == kNotFound) return GetHeap()->undefined_value();
Index: test/cctest/test-dictionary.cc
diff --git a/test/cctest/test-dictionary.cc b/test/cctest/test-dictionary.cc
index
15a854b363c42e613cb09593f81d196525712060..ffb0ad81bb5730cbb679510789b468ee7c655cfb
100644
--- a/test/cctest/test-dictionary.cc
+++ b/test/cctest/test-dictionary.cc
@@ -38,6 +38,7 @@
using namespace v8::internal;
+
TEST(ObjectHashTable) {
v8::HandleScope scope;
LocalContext context;
@@ -66,7 +67,8 @@ TEST(ObjectHashTable) {
CHECK_EQ(table->NumberOfDeletedElements(), 1);
CHECK_EQ(table->Lookup(*a), HEAP->undefined_value());
- // Keys should map back to their respective values.
+ // Keys should map back to their respective values and also should get
+ // an identity hash code generated.
for (int i = 0; i < 100; i++) {
Handle<JSObject> key = FACTORY->NewJSArray(7);
Handle<JSObject> value = FACTORY->NewJSArray(11);
@@ -74,12 +76,67 @@ TEST(ObjectHashTable) {
CHECK_EQ(table->NumberOfElements(), i + 1);
CHECK_NE(table->FindEntry(*key), ObjectHashTable::kNotFound);
CHECK_EQ(table->Lookup(*key), *value);
+ CHECK_NE(key->GetIdentityHash(OMIT_CREATION), HEAP->undefined_value());
+ }
+
+ // Keys never added to the map which already have an identity hash
+ // code should not be found.
+ for (int i = 0; i < 100; i++) {
+ Handle<JSObject> key = FACTORY->NewJSArray(7);
+ CHECK(!key->GetIdentityHash(ALLOW_CREATION)->IsFailure());
+ CHECK_NE(key->GetIdentityHash(OMIT_CREATION), HEAP->undefined_value());
+ CHECK_EQ(table->FindEntry(*key), ObjectHashTable::kNotFound);
+ CHECK_EQ(table->Lookup(*key), HEAP->undefined_value());
}
- // Keys never added to the map should not be found.
- for (int i = 0; i < 1000; i++) {
- Handle<JSObject> o = FACTORY->NewJSArray(100);
- CHECK_EQ(table->FindEntry(*o), ObjectHashTable::kNotFound);
- CHECK_EQ(table->Lookup(*o), HEAP->undefined_value());
+ // Keys that don't have an identity hash should not be found and also
+ // should not get an identity hash code generated.
+ for (int i = 0; i < 100; i++) {
+ Handle<JSObject> key = FACTORY->NewJSArray(7);
+ CHECK_EQ(table->Lookup(*key), HEAP->undefined_value());
+ CHECK_EQ(key->GetIdentityHash(OMIT_CREATION), HEAP->undefined_value());
}
}
+
+
+#ifdef DEBUG
+TEST(ObjectHashSetCausesGC) {
+ v8::HandleScope scope;
+ LocalContext context;
+ Handle<ObjectHashSet> table = FACTORY->NewObjectHashSet(1);
+ Handle<JSObject> key = FACTORY->NewJSArray(0);
+
+ // Simulate a full heap so that generating an identity hash code
+ // in subsequent calls will request GC.
+ FLAG_gc_interval = 0;
+
+ // Calling Contains() should not cause GC ever.
+ CHECK(!table->Contains(*key));
+
+ // Calling Remove() should not cause GC ever.
+ CHECK(!table->Remove(*key)->IsFailure());
+
+ // Calling Add() should request GC by returning a failure.
+ CHECK(table->Add(*key)->IsRetryAfterGC());
+}
+#endif
+
+
+#ifdef DEBUG
+TEST(ObjectHashTableCausesGC) {
+ v8::HandleScope scope;
+ LocalContext context;
+ Handle<ObjectHashTable> table = FACTORY->NewObjectHashTable(1);
+ Handle<JSObject> key = FACTORY->NewJSArray(0);
+
+ // Simulate a full heap so that generating an identity hash code
+ // in subsequent calls will request GC.
+ FLAG_gc_interval = 0;
+
+ // Calling Lookup() should not cause GC ever.
+ CHECK(table->Lookup(*key)->IsUndefined());
+
+ // Calling Put() should request GC by returning a failure.
+ CHECK(table->Put(*key, *key)->IsRetryAfterGC());
+}
+#endif
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
