Bug 194268

Summary: Safari does not free ArrayBuffers being transferred from webworker to main thread
Product: WebKit Reporter: Sanjay Kumar <hypertree>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Critical CC: cdumez, dino, keith_miller, lee, mike, rniwa, saam, simon.fraser, webkit-bug-importer, ysuzuki, zac.spitzer
Priority: P2 Keywords: InRadar
Version: Safari 12   
Hardware: iPhone / iPad   
OS: iOS 12   
See Also: https://bugs.webkit.org/show_bug.cgi?id=172790
https://bugs.webkit.org/show_bug.cgi?id=203990
Attachments:
Description Flags
tests none

Sanjay Kumar
Reported 2019-02-04 18:40:39 PST
Safari frequently runs out of memory running webapps using WebGL libraries such as Mapbox GL-JS and Tangram that transfer large number of araybuffers from webworker to main thread. This was previously reviewed here and mistakenly marked as RESOLVED/WORKSFORME: https://bugs.webkit.org/show_bug.cgi?id=172790#c10 Mapbox engineer has provided a small example that clearly shows Safari memory usage growing indefinitely as Safari never garbage collects ArrayBuffrs allocated in the webworker and transferred to foreground thread. Example can be found here (plain simple few lines of JavaScript, no WebGL): https://measuredweighed.github.io/mapboxgl-js-mem-usage/arraybuffer-test.html This example also crashes Firefox but rcent version of Chrome is able to garbage collect and keep memory usage down to 1 GB. Apparently such transfer of arraybuffers is core to WbeGL applications and expected to work. Safari users of my app have suffered for years and it impossible to release our App for iOS without this issue being fixed. It seems end is in sight since core issue has been identified.
Attachments
tests (2.12 KB, application/zip)
2019-04-03 19:03 PDT, Dean Jackson
no flags
Radar WebKit Bug Importer
Comment 1 2019-02-05 10:02:26 PST
kleggerkoder
Comment 2 2019-03-26 18:52:52 PDT
Any timelines on this bug fix?
Dean Jackson
Comment 3 2019-04-02 16:35:12 PDT
Taking a look at this now.
Dean Jackson
Comment 4 2019-04-03 19:02:40 PDT
I spent a fair bit of time on this and think the issue is fixed, at least on tip-of-tree. Attached two test cases I made (modifications to the one linked above). They allocate some ArrayBuffers in the worker, and then transfer them to the main thread via postMessage. The main thread only keeps references to a small number of buffers. The allocation timeline in the Web Inspector shows garbage collection triggering, and only a fixed number of ArrayBuffers being retained (the ones I'm keeping a reference to). I also tried with Instruments and the Allocations widget. It also showed that while a large number of ArrayBuffers were created, they were all reclaimed over time. Lastly I put printf statements in JavaScriptCore for ArrayBuffer's constructor and destructor. Each of the large ArrayBuffers allocated in the Worker was eventually reclaimed (and very soon after the references were dropped, because they were taking up a lot of memory). [Side note: I was not sure if it should be "each... was" or "each were". Most resources say "was" (s), but many say "were" (pl) is acceptable too.]
Dean Jackson
Comment 5 2019-04-03 19:03:05 PDT
Dean Jackson
Comment 6 2019-04-03 19:05:16 PDT
I also tried the original test with my printf statements and confirmed that the ArrayBuffers are being deallocated.
Sanjay Kumar
Comment 7 2019-04-04 04:38:07 PDT
It seems fix was included in ios 12.2 because when I tested a few weeks back my Web App was no longer crashing even if I left it running for extended time. Earlier it would crash in 5-10 minutes on low memory devices. This is great news and great relief to all of us WebGL users and Mapbox users - we can think of iOS now as a viable platform for Web Apps thanks to this fix. Thank you Apple/Webkit team !
Ryosuke Niwa
Comment 8 2019-04-04 13:41:09 PDT
(In reply to Sanjay Kumar from comment #7) > It seems fix was included in ios 12.2 because when I tested a few weeks back > my Web App was no longer crashing even if I left it running for extended > time. Earlier it would crash in 5-10 minutes on low memory devices. That's great! > This is great news and great relief to all of us WebGL users and Mapbox > users - we can think of iOS now as a viable platform for Web Apps thanks to > this fix. Thank you Apple/Webkit team ! Thank YOU for reporting the issue you saw. We always appreciate good bug reports on our products :)
Sanjay Kumar
Comment 9 2019-05-19 02:40:57 PDT
I am sorry but the problem is back in iOS 12.3. test is still available @: https://measuredweighed.github.io/mapboxgl-js-mem-usage/arraybuffer-test.html If you run the test for a few minutes on an IPAD 5 with 2 GB RAM and iOS 12.3 the page crashes in seconds. memory usage grows quickly and iOS Safari kills the web page almost immediately. Test does not crash on my iPhone 8 plus w/ iOS version 12.2. It also does not crash on Safari Technology Preview 81 or version 82 on High Sierra. So something went wrong in iOS 12.3. Please fix. Thanks.
Sanjay Kumar
Comment 10 2019-08-03 09:22:31 PDT
I checked further. It seems crash I am seeing does not have to do this with specific bug which indeed seems fixed. It seems Safari has some general limit of overall memory use and we are running into that due to memory bloat. So I am marking this bug as fixed and opening a new bug for Mapbox. Since I am seeing crashes only on iOS Safari so it may still have to do with webkit but probably its not this bug.
Ryosuke Niwa
Comment 11 2019-08-05 00:40:00 PDT
Thanks for the follow up!
Note You need to log in before you can comment on or make changes to this bug.