Commit 84b60253 authored by Ross McIlroy's avatar Ross McIlroy Committed by Commit Bot

Revert "[IdentityMap] Fix size if GC short-cuts objects."

This reverts commit d58bb2dc.

Reason for revert: New test breaks on optimize-for-size:
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20-%20debug/builds/16469/steps/OptimizeForSize/logs/GCShortCutting

Original change's description:
> [IdentityMap] Fix size if GC short-cuts objects.
> 
> BUG=chromium:704132
> 
> Change-Id: I6146c907d4f26147676f7dde4974c44fe541e8fe
> Reviewed-on: https://chromium-review.googlesource.com/541362
> Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#46059}

TBR=rmcilroy@chromium.org,mstarzinger@chromium.org

Change-Id: Ib2ba207dcc1b3193d3645090e9c0a9676f38c353
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:704132
Reviewed-on: https://chromium-review.googlesource.com/541224Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46061}
parent 6c7304b0
......@@ -243,13 +243,6 @@ void IdentityMapBase::Rehash() {
for (auto pair : reinsert) {
int index = InsertKey(pair.first);
DCHECK_GE(index, 0);
if (values_[index] != nullptr) {
// During GC some objects have been shortcut to point to the same
// underlying object - we can only chose one of the entries so keep the
// first one seen and reduce the size of the map appropriately
// (see crbug.com/704132 for details).
size_--;
}
values_[index] = pair.second;
}
}
......
......@@ -187,6 +187,7 @@ class IdentityMapTester : public HandleAndZoneScope {
void Rehash() { map.Rehash(); }
};
TEST(Find_smi_not_found) {
IdentityMapTester t;
for (int i = 0; i < 100; i++) {
......@@ -672,6 +673,7 @@ TEST(Collisions_7) { CollisionTest(7); }
TEST(Resize) { CollisionTest(9, false, true); }
TEST(Rehash) { CollisionTest(11, true, false); }
TEST(ExplicitGC) {
IdentityMapTester t;
Handle<Object> num_keys[] = {t.num(2.1), t.num(2.4), t.num(3.3), t.num(4.3),
......@@ -693,6 +695,7 @@ TEST(ExplicitGC) {
}
}
TEST(CanonicalHandleScope) {
Isolate* isolate = CcTest::i_isolate();
Heap* heap = CcTest::heap();
......@@ -772,54 +775,5 @@ TEST(CanonicalHandleScope) {
}
}
TEST(GCShortCutting) {
IdentityMapTester t;
Isolate* isolate = CcTest::i_isolate();
Factory* factory = isolate->factory();
const int kDummyValue = 0;
for (int i = 0; i < 16; i++) {
// Insert a varying number of Smis as padding to ensure some tests straddle
// a boundary where the thin string short cutting will cause size_ to be
// greater to capacity_ if not corrected by IdentityMap
// (see crbug.com/704132).
for (int j = 0; j < i; j++) {
t.map.Set(t.smi(j), reinterpret_cast<void*>(kDummyValue));
}
Handle<String> thin_string =
factory->NewStringFromAsciiChecked("thin_string");
Handle<String> internalized_string =
factory->InternalizeString(thin_string);
DCHECK_IMPLIES(FLAG_thin_strings, thin_string->IsThinString());
DCHECK_NE(*thin_string, *internalized_string);
// Insert both keys into the map.
t.map.Set(thin_string, &thin_string);
t.map.Set(internalized_string, &internalized_string);
// Do an explicit, real GC, this should short-cut the thin string to point
// to the internalized string.
t.heap()->CollectGarbage(i::NEW_SPACE,
i::GarbageCollectionReason::kTesting);
DCHECK_IMPLIES(FLAG_thin_strings && !FLAG_optimize_for_size,
*thin_string == *internalized_string);
// Check that getting the object points to one of the handles.
void** thin_string_entry = t.map.Get(thin_string);
CHECK(*thin_string_entry == &thin_string ||
*thin_string_entry == &internalized_string);
void** internalized_string_entry = t.map.Get(internalized_string);
CHECK(*internalized_string_entry == &thin_string ||
*internalized_string_entry == &internalized_string);
// Trigger resize.
for (int j = 0; j < 16; j++) {
t.map.Set(t.smi(j + 16), reinterpret_cast<void*>(kDummyValue));
}
t.map.Clear();
}
}
} // namespace internal
} // namespace v8
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment