Under some circumstances, we run out of memory Thread 0 Crashed ↩:: Dispatch queue: com.apple.main-thread 0 JavaScriptCore 0x18ccfef34 bmalloc_allocate_impl_impl_slow + 60 (/Library/Caches/com.apple.xbs/Sources/bmalloc/WebKit-7613.1.16.0.4/Source/bmalloc/libpas/src/libpas/pas_utils.h:186) 1 JavaScriptCore 0x18ccf7f78 bmalloc_allocate_impl_casual_case + 272 (/Library/Caches/com.apple.xbs/Sources/bmalloc/WebKit-7613.1.16.0.4/Source/bmalloc/libpas/src/libpas/pas_try_allocate_intrinsic.h:173) 2 WebCore 0x191a75728 WebCore::combineSegmentsData(WTF::Vector<WebCore::FragmentedSharedBuffer::DataSegmentVectorEntry, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, unsigned long) + 72 (/System/Volumes/Data/SWE/iOS/BuildRoots/BuildRoot949/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS15.4.Internal.sdk/usr/local/include/wtf/FastMalloc.h:219) 3 WebCore 0x191a7566c WebCore::FragmentedSharedBuffer::makeContiguous() const + 280 (/Library/Caches/com.apple.xbs/Sources/WebCore/WebKit-7613.1.16.0.4/Source/WebCore/./platform/SharedBuffer.cpp:120) 4 WebCore 0x1918c3e34 WebCore::CachedRawResource::updateBuffer(WebCore::FragmentedSharedBuffer const&) + 140 (/Library/Caches/com.apple.xbs/Sources/WebCore/WebKit-7613.1.16.0.4/Source/WebCore/./loader/cache/CachedRawResource.cpp:71) 5 WebCore 0x191890b78 WebCore::SubresourceLoader::didReceiveBuffer(WebCore::FragmentedSharedBuffer const&, long long, WebCore::DataPayloadType) + 252 (/Library/Caches/com.apple.xbs/Sources/WebCore/WebKit-7613.1.16.0.4/Source/WebCore/./loader/SubresourceLoader.cpp:545) 6 WebKit 0x18fb7e344 WebKit::WebResourceLoader::didReceiveData(IPC::SharedBufferCopy const&, long long) + 296 (/Library/Caches/com.apple.xbs/Sources/WebKit/WebKit-7613.1.16.0.4/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp:221) 7 WebKit 0x18fd1e340 WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::Decoder&) + 380 (/Library/Caches/com.apple.xbs/Sources/WebKit/WebKit-7613.1.16.0.4/Source/WebKit/Platform/IPC/HandleMessage.h:125) 8 WebKit 0x18f7856d0 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 788 (/Library/Caches/com.apple.xbs/Sources/WebKit/WebKit-7613.1.16.0.4/Source/WebKit/Platform/IPC/Connection.cpp:1080) 9 WebKit 0x18f787e74 WTF::Detail::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_13, void>::call() + 252 (/Library/Caches/com.apple.xbs/Sources/WebKit/WebKit-7613.1.16.0.4/Source/WebKit/Platform/IPC/Connection.cpp:1194) 10 JavaScriptCore 0x18cc9bb94 WTF::RunLoop::performWork() + 200 (/Library/Caches/com.apple.xbs/Binaries/WTF/install/Root/usr/local/include/wtf/Function.h:82) 11 JavaScriptCore 0x18cc9c930 WTF::RunLoop::performWork(void*) + 36 (/Library/Caches/com.apple.xbs/Sources/WTF/WebKit-7613.1.16.0.4/Source/WTF/wtf/cf/RunLoopCF.cpp:46) While we can't tell which sites this is occurring with, when using Speedtest.net we get very close to the 1GB memory limit and sometimes ever going over it. One possible explanation for this high memory usage is that whenever we receive a new partial segment for a XHRRequest, the FragmentedSharedBuffer is flattened by CachedRawResource before being passed to the various listener. Prior bug 233030 this flattening was also occurring (though silently) when SharedBuffer::data() method was called. Bug 233030 changed the signature of the crash. But not flattening repeatedly the SharedBuffer and only doing it for listener actually needed it we can lower the memory usage (though it may be at the expense of small speed loss as flattening of the SharedBuffer could be done multiple times for the same SharedBuffer).
rdar://87596724
Created attachment 452161 [details] WIP
Created attachment 452169 [details] WIP
Created attachment 452172 [details] WIP
Created attachment 452222 [details] Patch
Comment on attachment 452222 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452222&action=review > Source/WebCore/ChangeLog:11 > + FragmentedSharedBuffer is receieved from the network process, it is proactively coalesced received > Source/WebCore/loader/cache/CachedRawResource.cpp:118 > + if (auto incrementalData = calculateIncrementalDataChunk(contiguousData)) { this should be using data rather than contiguousData. Otherwise the client would have received each individual segment, and then a SharedBufferDataView restricting the content to just the last segment received but that actually takes a reference to the entire content. This could potentially means we keep a reference for longer than needed. > Source/WebCore/loader/cache/CachedScript.cpp:87 > m_scriptHash = m_script.impl()->hash(); If the CachedScript received fragmented content, finishLoading would have been called. This version has dropped the makeContiguous to m_data in finishLoading, this could potentially lead to multiple call to makeContiguous on the same buffer
(In reply to Jean-Yves Avenard [:jya] from comment #6) > Comment on attachment 452222 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=452222&action=review > > > Source/WebCore/ChangeLog:11 > > + FragmentedSharedBuffer is receieved from the network process, it is proactively coalesced > > received > > > Source/WebCore/loader/cache/CachedRawResource.cpp:118 > > + if (auto incrementalData = calculateIncrementalDataChunk(contiguousData)) { > > this should be using data rather than contiguousData. > > Otherwise the client would have received each individual segment, and then a > SharedBufferDataView restricting the content to just the last segment > received but that actually takes a reference to the entire content. This > could potentially means we keep a reference for longer than needed. Okay. Can we fix that in a future patch? This one keeps the original behavior of coalescing and storing in m_data, then passing m_data in. > > Source/WebCore/loader/cache/CachedScript.cpp:87 > > m_scriptHash = m_script.impl()->hash(); > > If the CachedScript received fragmented content, finishLoading would have > been called. This version has dropped the makeContiguous to m_data in > finishLoading, this could potentially lead to multiple call to > makeContiguous on the same buffer Not sure I understand here; CachedScript::finishLoading() still coalesces the incoming buffer.
Comment on attachment 452222 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452222&action=review > Source/WebCore/loader/cache/CachedRawResource.cpp:70 > + m_data = data.copy(); forgot to mention, if you take a copy rather than a reference to data ; you must keep a strong ref to data above, as a call to notifyClientsDataWasReceived will result in a re-entrant call to the CachedRawResource parent that will destroy data.
Created attachment 452294 [details] Patch
Created attachment 452295 [details] Patch
Committed r290005 (247391@main): <https://commits.webkit.org/247391@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452295 [details].