Once saved to disk, then mmap'd data of service worker scripts back to the ServiceWorker process, in order to save dirty memory. This is similar to what we do in the network cache implementation.
Created attachment 424818 [details] Patch
Created attachment 424855 [details] Patch
Created attachment 424857 [details] Patch
<rdar://75637679>
Comment on attachment 424857 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424857&action=review > Source/WebCore/platform/SharedBuffer.cpp:224 > + unsigned count = 0; > + for (auto it = begin(); it != end(); ++it) > + ++count; > + return count; This can be written like this: return std::count(begin(), end()); But also, not sure we want this function. I think hasOneSegment would be a better function to add. It would be something like this: auto it = begin(); return it != end() && ++it == end(); > Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:117 > + if (buffer.numberOfSegments() != 1) This is an inefficient way to check If there is exactly one segment if we have a buffer with a lot segments in it!
(In reply to Darin Adler from comment #5) > Comment on attachment 424857 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424857&action=review > > > Source/WebCore/platform/SharedBuffer.cpp:224 > > + unsigned count = 0; > > + for (auto it = begin(); it != end(); ++it) > > + ++count; > > + return count; > > This can be written like this: > > return std::count(begin(), end()); > > But also, not sure we want this function. I think hasOneSegment would be a > better function to add. It would be something like this: > > auto it = begin(); > return it != end() && ++it == end(); > > > Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:117 > > + if (buffer.numberOfSegments() != 1) > > This is an inefficient way to check If there is exactly one segment if we > have a buffer with a lot segments in it! Ok. Will address. Please don't review this iteration further. This patch is very outdated and I will upload a fresh version shortly.
(In reply to Chris Dumez from comment #6) > Please don't review this iteration further. This patch is > very outdated and I will upload a fresh version shortly. No worries, I won’t.
Created attachment 424908 [details] Patch
Comment on attachment 424908 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424908&action=review r=me > Source/WebCore/ChangeLog:13 > + scripts to the ServiceWorker processes as ShareableResource handles so that the dirty memory usage so that => in order to reduce > Source/WebCore/ChangeLog:16 > + No new tests, no Web-facing behavior change, just a decrease dirty memory use in the ServiceWorker decrease => decrease in
Comment on attachment 424908 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424908&action=review > Source/WebCore/ChangeLog:19 > + memory usage goes from ~440MB to ~230MB in both the cold and warm cases ("Cold" meaning that the I wonder if, in the cold case, executing the service worker script causes JavaScriptCore to retain a large dirty memory string. Maybe that explains the remaining 230MB?
(In reply to Geoffrey Garen from comment #10) > Comment on attachment 424908 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424908&action=review > > > Source/WebCore/ChangeLog:19 > > + memory usage goes from ~440MB to ~230MB in both the cold and warm cases ("Cold" meaning that the > > I wonder if, in the cold case, executing the service worker script causes > JavaScriptCore to retain a large dirty memory string. Maybe that explains > the remaining 230MB? 230MB remain in the WebProcess in both the cold and warm cases. This seems to indicate both 100MB scripts are still kept in memory in the WebProcess in one place. Looking at the code, I *think* it must in JavaScriptCore indeed. I can try and find out why in a follow-up.
Created attachment 424927 [details] Patch
Maybe a garbage-collectable string that just hasn’t been collected yet?
(In reply to Darin Adler from comment #13) > Maybe a garbage-collectable string that just hasn’t been collected yet? Maybe but I did send a memory pressure warning before collecting memory measurements. The memory pressure warning *should* have triggered a sync GC I think.
(In reply to Chris Dumez from comment #14) > (In reply to Darin Adler from comment #13) > > Maybe a garbage-collectable string that just hasn’t been collected yet? > > Maybe but I did send a memory pressure warning before collecting memory > measurements. The memory pressure warning *should* have triggered a sync GC > I think. Note that the scripts I used are very simple: large.js: ``` importScripts('large-sub.js'); a = 1; /* a 100MB comment */ ``` large-sub.js: ``` b = 2; /* a 100 MB comment */ ```
Committed r275375: <https://commits.webkit.org/r275375> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424927 [details].
(In reply to Darin Adler from comment #13) > Maybe a garbage-collectable string that just hasn’t been collected yet? Turns out we were not clearing the JS code cache or doing GC in worker threads on memory pressure. I followed up in Bug 224110.