Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting explicit autorelease frequency #579

Conversation

adamwulf
Copy link
Contributor

@adamwulf adamwulf commented Feb 1, 2023

I was seeing large memory growth when offline with lots of items waiting to be flushed to the mp service. i had ~5000 events queued while offline, and the work done on these dispatch queues wasn't getting cleaned up, causing an explosion in otherwise collectable memory.

my understanding is that DispatchQueue() will default to autoreleaseFrequency=.never, which can leave those autoreleased objects hanging around in memory for undefined amounts of time.

@adamwulf
Copy link
Contributor Author

adamwulf commented Feb 2, 2023

Related to #563

@jaredmixpanel
Copy link
Contributor

@adamwulf can you please pull in the latest changes from master? they include some CI config changes that will fix your CI failure.

@jaredmixpanel jaredmixpanel changed the base branch from master to jared-multi-pr-pick March 1, 2023 00:06
@jaredmixpanel jaredmixpanel merged commit fa699e5 into mixpanel:jared-multi-pr-pick Mar 1, 2023
@adamwulf
Copy link
Contributor Author

adamwulf commented Mar 1, 2023

Ah! sorry, saw this a bit too late. glad it was still able to get merged in. Thanks!

@jaredmixpanel
Copy link
Contributor

@adamwulf no worries. we'll get a new release out soon.

@RamblinWreck77
Copy link
Contributor

Late to the party on this one, so what exactly will this change do? Does autoreleaseFrequency: .workItem effectively add a timeout to dispatched work when offline/events can't send?

@adamwulf
Copy link
Contributor Author

adamwulf commented Mar 1, 2023

autoreleaseFrequency: .workItem will essentially wrap every block in the queue with an autoreleasepool { ... }. this ensures the pool is drained when the block completes on the queue, which makes memory releases more deterministic. it doesn't have any effect on timing/delay/etc

@RamblinWreck77
Copy link
Contributor

RamblinWreck77 commented Mar 1, 2023

Thanks! I need to learn more about this it seems.

https://developer.apple.com/documentation/dispatch/dispatchqueue/2300059-init

Seems to suggest that the default is .inherit, not .never?

Some background stuff I've found:

https://stackoverflow.com/questions/59019950/what-is-autorelease-frequency-and-target-while-creating-dispatch-queue

Re autorelease frequency, see the description of the possible values. It dictates when an autorelease pool is drained. An autorelease object is a type of memory management available in Objective-C (see Advanced Memory Management Programming Guide). But this is of diminished relevance in Swift, really only an issue if your Swift code is calling some Objective-C code that is creating autorelease objects, which is increasingly uncommon. And even if you are dealing with Objective-C autorelease objects, the default behavior is generally preferable. A substantive discussion of autorelease objects is probably beyond the scope of this question, but, in short, this parameter is rarely needed, especially not in Swift.

https://stackoverflow.com/questions/38884418/autoreleasefrequency-on-dispatchqueue-in-swift-3

So the mixpanel-swift lib is pure swift right, so maybe this won't actually effect things? Either way I don't think this change will hurt anything, just trying to to understand it incase we see weird behavior in the future.

@adamwulf
Copy link
Contributor Author

adamwulf commented Mar 1, 2023

my understanding is that the pool will eventually be drained because of .inherit, but just it lets the system/something else decide when, and it opens up to a problem of it not being drained soon enough because of some specific scenario. in our case, i believe it was related to #583, which was causing the queue to be saturated and run constantly, so then it never drained its pool.

i believe there are still cases in Swift that are bridged to Obj-C behinds the scenes, so even in a pure swift codebase there may still be obj-c objects being created/destroyed. Dictionary may transparently wrap NSDictionary in some cases, etc. and even though the MP library is Swift, the apps it integrates with might not be and could be passing in bridged objects to the event properties/etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants