Title: [247368] trunk/Tools
- Revision
- 247368
- Author
- ysuz...@apple.com
- Date
- 2019-07-11 14:56:32 -0700 (Thu, 11 Jul 2019)
Log Message
Flaky API Test TestWTF.bmalloc.ScavengedMemoryShouldBeReused
https://bugs.webkit.org/show_bug.cgi?id=199524
<rdar://problem/52783816>
Reviewed by Saam Barati.
This test is white-box one and it has strong assumption how IsoHeap allocates pages.
But this test has several problems.
1. IsoPage::numObjects is not the exact number of how many we allocate objects. This
number is calculated by pageSize / sizeof(T), and this does not account the header
size of IsoPage. So, # of objects per IsoPage is less than numObjects. Since sizeof(double)
is very small, we can have many objects in one IsoPage. As a result, we need a large
bitmap in IsoPage. This reduces # of objects in IsoPage largely. So, `ptrs.size()` becomes
less than numObjects.
2. We now have lower tier of allocation in IsoHeap. It means that we allocate 8 objects in
shared page (page is shared, but memory is pinned for a specific type) before using IsoHeap's
page. This also makes the intention of this test wrong.
Due to (1), we access OoB of ptrs vector, passing a garbage to IsoHeap::deallocate, and crashing.
We make this test robust while we still keep this test white-box one to test the critical feature
of IsoHeap. We first exhaust lower tier of IsoHeap, and after that, start testing the memory. We
allocate many pointers, deallocate them, allocate one pointer while keeping pointers in the lower
tier live, and check whether the deallocated memory is reused.
* TestWebKitAPI/Tests/WTF/bmalloc/IsoHeap.cpp:
(TEST):
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (247367 => 247368)
--- trunk/Tools/ChangeLog 2019-07-11 21:51:24 UTC (rev 247367)
+++ trunk/Tools/ChangeLog 2019-07-11 21:56:32 UTC (rev 247368)
@@ -1,3 +1,35 @@
+2019-07-11 Yusuke Suzuki <ysuz...@apple.com>
+
+ Flaky API Test TestWTF.bmalloc.ScavengedMemoryShouldBeReused
+ https://bugs.webkit.org/show_bug.cgi?id=199524
+ <rdar://problem/52783816>
+
+ Reviewed by Saam Barati.
+
+ This test is white-box one and it has strong assumption how IsoHeap allocates pages.
+ But this test has several problems.
+
+ 1. IsoPage::numObjects is not the exact number of how many we allocate objects. This
+ number is calculated by pageSize / sizeof(T), and this does not account the header
+ size of IsoPage. So, # of objects per IsoPage is less than numObjects. Since sizeof(double)
+ is very small, we can have many objects in one IsoPage. As a result, we need a large
+ bitmap in IsoPage. This reduces # of objects in IsoPage largely. So, `ptrs.size()` becomes
+ less than numObjects.
+
+ 2. We now have lower tier of allocation in IsoHeap. It means that we allocate 8 objects in
+ shared page (page is shared, but memory is pinned for a specific type) before using IsoHeap's
+ page. This also makes the intention of this test wrong.
+
+ Due to (1), we access OoB of ptrs vector, passing a garbage to IsoHeap::deallocate, and crashing.
+
+ We make this test robust while we still keep this test white-box one to test the critical feature
+ of IsoHeap. We first exhaust lower tier of IsoHeap, and after that, start testing the memory. We
+ allocate many pointers, deallocate them, allocate one pointer while keeping pointers in the lower
+ tier live, and check whether the deallocated memory is reused.
+
+ * TestWebKitAPI/Tests/WTF/bmalloc/IsoHeap.cpp:
+ (TEST):
+
2019-07-11 Pablo Saavedra <psaave...@igalia.com>
[WPE][GTK] Build failure with ENABLE_ACCESSIBILITY=OFF
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/bmalloc/IsoHeap.cpp (247367 => 247368)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/bmalloc/IsoHeap.cpp 2019-07-11 21:51:24 UTC (rev 247367)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/bmalloc/IsoHeap.cpp 2019-07-11 21:56:32 UTC (rev 247368)
@@ -313,10 +313,17 @@
static IsoHeap<double> heap;
auto run = [] (unsigned numPagesToCommit) {
- auto* ptr1 = heap.allocate();
-
+ std::vector<void*> lowerTierPtrs;
std::vector<void*> ptrs;
+ // Let's exhaust the capacity of the lower tier.
+ for (unsigned i = 0; i < IsoPage<decltype(heap)::Config>::numObjects; ++i) {
+ void* ptr = heap.allocate();
+ CHECK(ptr);
+ lowerTierPtrs.push_back(ptr);
+ }
+
+ // After that, allocating pointers in the upper tier.
for (unsigned i = 0; ;i++) {
void* ptr = heap.allocate();
CHECK(ptr);
@@ -325,24 +332,33 @@
break;
}
- std::set<void*> uniquedPtrs = toptrset(ptrs);
+ std::set<void*> uniquedPtrsOfUpperTiers = toptrset(ptrs);
+ CHECK(ptrs.size() == uniquedPtrsOfUpperTiers.size());
- heap.deallocate(ptr1);
- for (unsigned i = 0; i < IsoPage<decltype(heap)::Config>::numObjects - 1; i++) {
- heap.deallocate(ptrs[i]);
- uniquedPtrs.erase(ptrs[i]);
+ std::set<void*> uniquedPtrs = uniquedPtrsOfUpperTiers;
+ for (void* ptr : lowerTierPtrs)
+ uniquedPtrs.insert(ptr);
+
+ // We do keep pointers in the lower tier while deallocating pointers in the upper tier.
+ // Then, after the scavenge, the pages of the upper tier should be reused.
+
+ for (void* ptr : ptrs) {
+ heap.deallocate(ptr);
+ uniquedPtrs.erase(ptr);
}
scavenge();
assertHasOnlyObjects(heap, uniquedPtrs);
- // FIXME: This only seems to pass when lldb is attached but the scavenger thread isn't running...
- // see: https://bugs.webkit.org/show_bug.cgi?id=198384
- // auto* ptr2 = heap.allocate();
- // CHECK(ptr1 == ptr2);
+ auto* ptr2 = heap.allocate();
+ CHECK(uniquedPtrsOfUpperTiers.find(ptr2) != uniquedPtrsOfUpperTiers.end());
+ heap.deallocate(ptr2);
+
+ for (void* ptr : lowerTierPtrs)
+ heap.deallocate(ptr);
};
- run(2);
+ run(5);
}
template<size_t N>
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
