Title: [289019] branches/safari-613-branch/Source/WebCore
- Revision
- 289019
- Author
- [email protected]
- Date
- 2022-02-02 16:59:08 -0800 (Wed, 02 Feb 2022)
Log Message
Cherry-pick r288667. rdar://problem/87830583
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: https://svn.webkit.org/repository/webkit/trunk@288667 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-613-branch/Source/WebCore/ChangeLog (289018 => 289019)
--- branches/safari-613-branch/Source/WebCore/ChangeLog 2022-02-03 00:58:19 UTC (rev 289018)
+++ branches/safari-613-branch/Source/WebCore/ChangeLog 2022-02-03 00:59:08 UTC (rev 289019)
@@ -1,5 +1,76 @@
2022-02-02 Russell Epstein <[email protected]>
+ Cherry-pick r288667. rdar://problem/87830583
+
+ 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: https://svn.webkit.org/repository/webkit/trunk@288667 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-01-26 Jean-Yves Avenard <[email protected]>
+
+ 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-02-02 Russell Epstein <[email protected]>
+
Cherry-pick r288955. rdar://problem/88359001
Unreviewed, reverting r286988.
Modified: branches/safari-613-branch/Source/WebCore/loader/cache/CachedRawResource.cpp (289018 => 289019)
--- branches/safari-613-branch/Source/WebCore/loader/cache/CachedRawResource.cpp 2022-02-03 00:58:19 UTC (rev 289018)
+++ branches/safari-613-branch/Source/WebCore/loader/cache/CachedRawResource.cpp 2022-02-03 00:59:08 UTC (rev 289019)
@@ -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)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
