[iOS] Add autorelease pool for each run loop for JS Thread #27395
Conversation
facebook-github-bot
added
the
CLA Signed
| CFRunLoopPerformBlock(m_cfRunLoop, kCFRunLoopCommonModes, ^{ | ||
| // Create an autorelease pool each run loop to prevent memory footprint from growing too large, which can lead to performance problems. | ||
| @autoreleasepool { | ||
| func(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
This will impact pre-TurboModule system as well. The OOM is only when TM is used right? So I think the fix should be done somewhere around the caller of runAsync that's specific to TM?
In other words, this code doesn't leak memory pre-TM, why is that? If we can find the logic in the old system that prevents the leak, we should apply the same fix for TM. So this PR might not address the TM-specific reason for the leak.
There was a problem hiding this comment.
@fkgozali Actually, pre-TurboModule also has the issue, the reason why pre-TurboModule seems not have is it not creates giant autorelease objects, for example, TurboModule create one NSInvocation (autorelease object) for each method call, but pre-TurboModule only create one by cache. So pre-TurboModule just hidden the issue in @sjchmiela 's demo, but actually it also has if module method create many autorelease objects.
There was a problem hiding this comment.
but pre-TurboModule only create one by cache
I see, good catch. I think we should aim to get the caching ideally, but that may be out of scope for this issue. If you're up for some refactoring, that would be helpful :).
There was a problem hiding this comment.
@fkgozali has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
This pull request was successfully merged by @zhongwuzw in 948cbfd. When will my fix make it into a release? | Upcoming Releases |
Summary
Fixes #27327 , we need to create autorelease pool for each run loop in secondary thread, otherwise application may have memory issues. More details can refer to CreatingThreads
Changelog
[iOS] [Fixed] - Add autorelease pool for each run loop for JS Thread
Test Plan
Example can be found in #27327. No memory spikes any more.