Bug 194268 - Safari does not free ArrayBuffers being transferred from webworker to main thread
Summary: Safari does not free ArrayBuffers being transferred from webworker to main th...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari 12
Hardware: iPhone / iPad iOS 12
: P2 Critical
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-02-04 18:40 PST by Sanjay Kumar
Modified: 2020-01-08 12:17 PST (History)
11 users (show)

See Also:


Attachments
tests (2.12 KB, application/zip)
2019-04-03 19:03 PDT, Dean Jackson
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sanjay Kumar 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.
Comment 1 Radar WebKit Bug Importer 2019-02-05 10:02:26 PST
<rdar://problem/47821447>
Comment 2 kleggerkoder 2019-03-26 18:52:52 PDT
Any timelines on this bug fix?
Comment 3 Dean Jackson 2019-04-02 16:35:12 PDT
Taking a look at this now.
Comment 4 Dean Jackson 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.]
Comment 5 Dean Jackson 2019-04-03 19:03:05 PDT
Created attachment 366692 [details]
tests
Comment 6 Dean Jackson 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.
Comment 7 Sanjay Kumar 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 !
Comment 8 Ryosuke Niwa 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 :)
Comment 9 Sanjay Kumar 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.
Comment 10 Sanjay Kumar 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.
Comment 11 Ryosuke Niwa 2019-08-05 00:40:00 PDT
Thanks for the follow up!