REGRESSION(r287684) speedtest.net uses many GB of memory
https://bugs.webkit.org/show_bug.cgi?id=235615
rdar://87830583
Reviewed by Youenn Fablet.
The regression was introduced with r286937 and is a good example of
errors introduced when attempting to optimise things too early.
CachedRawResource::updateBuffer does a search in the accumulating
resource's SharedBuffer, search that was taking O(log(n)+1) prior r286937
where n is the number of DataView segments in the SharedBuffer.
This was simplified as a O(1) operation by using the combined contiguous
SharedBuffer instead.
However, that caused every single intermediary accumulated buffers to be
kept referenced by the XMLHttpRequest SharedBufferBuilder leading to
massive memory use.
In other words:
For each update, we did the following steps:
- Set m_data to a new big continuous chunk of data that stores all
received data
- Create a view of the new data as a SharedBuffer. This SharedBuffer
references the big continuous chunk above
- XHR stores a ref to the view, hence keep the big chunk alive.
Each XHR chunk, although small in data that can be accessed, is actually keeping in memory all temporary created m_data chunks.
Following this change, XHR will now only keeps a reference to the new DataSegment added since the last run rather than the entire previous content.
Fly-by: add some comments describing the running of the method.
* loader/cache/CachedRawResource.cpp:
(WebCore::CachedRawResource::updateBuffer):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@288667 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 466aeab..add7cda 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,36 @@
+2022-01-26 Jean-Yves Avenard <jya@apple.com>
+
+ REGRESSION(r287684) speedtest.net uses many GB of memory
+ https://bugs.webkit.org/show_bug.cgi?id=235615
+ rdar://87830583
+
+ Reviewed by Youenn Fablet.
+
+ The regression was introduced with r286937 and is a good example of
+ errors introduced when attempting to optimise things too early.
+ CachedRawResource::updateBuffer does a search in the accumulating
+ resource's SharedBuffer, search that was taking O(log(n)+1) prior r286937
+ where n is the number of DataView segments in the SharedBuffer.
+ This was simplified as a O(1) operation by using the combined contiguous
+ SharedBuffer instead.
+ However, that caused every single intermediary accumulated buffers to be
+ kept referenced by the XMLHttpRequest SharedBufferBuilder leading to
+ massive memory use.
+ In other words:
+ For each update, we did the following steps:
+ - Set m_data to a new big continuous chunk of data that stores all
+ received data
+ - Create a view of the new data as a SharedBuffer. This SharedBuffer
+ references the big continuous chunk above
+ - XHR stores a ref to the view, hence keep the big chunk alive.
+ Each XHR chunk, although small in data that can be accessed, is actually keeping in memory all temporary created m_data chunks.
+ Following this change, XHR will now only keeps a reference to the new DataSegment added since the last run rather than the entire previous content.
+
+ Fly-by: add some comments describing the running of the method.
+
+ * loader/cache/CachedRawResource.cpp:
+ (WebCore::CachedRawResource::updateBuffer):
+
2022-01-26 Diego Pino Garcia <dpino@igalia.com>
[WPE] Unreviewed, fix non-unified build after r288640
diff --git a/Source/WebCore/loader/cache/CachedRawResource.cpp b/Source/WebCore/loader/cache/CachedRawResource.cpp
index d336113..55c45be 100644
--- a/Source/WebCore/loader/cache/CachedRawResource.cpp
+++ b/Source/WebCore/loader/cache/CachedRawResource.cpp
@@ -62,19 +62,24 @@
if (m_inIncrementalDataNotify)
return;
+ // We need to keep a strong reference to both the SharedBuffer and the current CachedRawResource instance
+ // as notifyClientsDataWasReceived call may delete both.
CachedResourceHandle<CachedRawResource> protectedThis(this);
+ auto protectedData = Ref { data };
+
ASSERT(dataBufferingPolicy() == DataBufferingPolicy::BufferData);
m_data = data.makeContiguous();
+ // Notify clients only of the newly appended content since the last run.
auto previousDataSize = encodedSize();
- while (m_data->size() > previousDataSize) {
- auto incrementalData = m_data->getSomeData(previousDataSize);
+ while (data.size() > previousDataSize) {
+ auto incrementalData = data.getSomeData(previousDataSize);
previousDataSize += incrementalData.size();
SetForScope<bool> notifyScope(m_inIncrementalDataNotify, true);
notifyClientsDataWasReceived(incrementalData.createSharedBuffer());
}
- setEncodedSize(m_data->size());
+ setEncodedSize(data.size());
if (dataBufferingPolicy() == DataBufferingPolicy::DoNotBufferData) {
if (m_loader)
