Memory warning

[nikolovdragan]

I have a problem when using CoreStore. After I do an update (big transaction) on the db some objects are not destroyed and memory increases until a memory warning is received and the app is closed. What should I do for a better memory management in a transaction?

[JohnEstropia]

I'm not sure if this can be fixed within CoreStore. Each transaction already creates their own scratch NSManagedObjectContext, so after each transaction all unsaved data should be deallocated and objects in different contexts are likely return to their faulting state.

Otherwise, memory usage within beginAsynchronous(...) and beginSynchronous(...) closures will be unavoidable because the context needs to keep objects in memory before you save your pending inserts/updates. You can try to improve this by processing your data in manageable "chunks" to decrease interval between saves, something like

let jsonArray: [NSDictionary] = // ...
let chunkSize = 50
for i in jsonArray.startIndex.stride(through: jsonArray.endIndex, by: chunkSize) {

    let chunkArray = jsonArray[i ..< i.advancedBy(chunkSize, limit: jsonArray.endIndex)]
    CoreStore.beginAsynchronous { (transaction) in

        // ... process chunkArray

        transaction.commit()
    }
}

[milanhorvatovic]

I have a same problem with current version. I read your source code and I have one question to you about creating temporary contexts.

Why do you register for notification about changing in self temporary context ?

NSManagedObjectContext+Transaction.swift

internal func temporaryContextInTransactionWithConcurrencyType(concurrencyType: NSManagedObjectContextConcurrencyType) -> NSManagedObjectContext {
    ...
    context.setupForCoreStoreWithContextName("com.corestore.temporarycontext")
    ...
    return context
}

If I comment this line (96) the memory consumption is rapidly reduced without any changes in my source code.

@nikolovdragan could you try this changes and write your result?

[nikolovdragan]

@milanhorvatovic This didn't work for me. I commented the line and the memory gets increased after every transaction. For testing purposes I repeat the transaction multiple times. After each transaction the memory increases from 1MB to 7MB. I leave the app overnight and the memory gets from 50MB to 200MB and I get memory warnings until the app closes. I flush and the transaction and reset the internal context after commit. This seems to help with the memory usage but it's not enough to free all the memory.

[JohnEstropia]

@milanhorvatovic @nikolovdragan: Just to get it out of the way, do you happen to have NSZombies on? If not then read ahead...

@milanhorvatovic Removing that line is not a good idea as it's obtaining permanent IDs for your objects. Without that you will get errors in your other contexts if you are using NSFetchedResultControllers.

If doing that reduces memory consumption for you, I am guessing that you are loading a ton of objects in a single transaction. Around many objects are you processing in a single go?

@nikolovdragan If flushing and resetting the internal context does not free all memory in your case, then you are probably simply saving a lot of objects and there's nothing Core Data can do for you. How much data are you working with? Are you using NSFetchedResultsControllers or are you keeping objects in your own arrays?

[milanhorvatovic]

It's not good idea for rootSaving or main context, but for temporary contexts it isn't necessary.

[nikolovdragan]

@JohnEstropia I am not using NSFetchedResultsControllers, I haven't seen it used in CoreStore. I use my own dictionaries to save the objects temporarily. I am saving around 30000 objects in a single transaction. This is a requirement of the app, the changes have to become visible all at once, saving the data in pieces is not an option.

[ghost]

Guys, sorry for jumping in here, but you may find Apple's guide on this subject helpful. Moreover:

... accessing relationships between objects in both directions creates retain cycles. If you see growing memory usage during the import despite well-placed auto-release pools, watch out for this pitfall in the importer code.... source

I'm also importing lots of objects using this library, and I'm not hitting memory warning (However it could be I'm just lucky).

Good luck, guys!

[JohnEstropia]

↑Right, I forgot about that too. Thanks @AleksandarPetrov !

With CoreStore, that should only affect objects in the main context though. While it would occur within transactions, any retain cycles in the temporary context should be broken after the context gets saved/released. Anyway, @milanhorvatovic and @nikolovdragan, please check Aleksandar's link above if it helps you.

[JohnEstropia]

I'm closing this issue for now, if you find any other problems that may be worked around within CoreStore, feel free to reopen the ticket and continue the discussion :)

[milanhorvatovic]

Sorry for my delay, but I had too many duties. I don't programming iOS application for the first year, so your solution by adding @autorelease closures on strategic points it's irrelevant in this situation. I have @autorelease closure and another technics to managing memory on all point where it's necessary. But ok thank you for your suggestion.

I know my situation with CoreData isn't standard because I'm trying import into storage around 390K records with multiple indices. Also I know about memory management and holding too many unnecessary cached memory for "optimisation" inside the system implementation of CoreData. Any articles about import optimisations are with dataset up to 20K records. It's clear I will lose a lot of computing time, but not too many memory.

That all is my mess but on the other hand you have memory issue and strong reference cycle between managedContext and observerForWillSaveNotification. So @JohnEstropia please checks your NotificationObserver and then will write your conclusion. thanks

[JohnEstropia]

@milanhorvatovic 390,000 records per save is a LOT! As you have seen with other articles, with CoreData the "accepted" upper limit should be around 10,000 objects or so. Your dataset is 100 times higher than that, and you are very likely to hit hardware limits like this.

As I have mentioned above, the background contexts need to retain objects in order to save them, and the observerForWillSaveNotification is needed to finalize temporary IDs. We cannot omit these without breaking anything.

If you have evidence that a retain cycle is actually happening, please do share. Do note that just because you have a memory spike on one method does not mean there is a reference cycle. Normally the memory would clear up afterwards. I guarantee that we benchmark our apps regularly and we have not seen memory leaks in CoreStore so far.

With a dataset that size, I really recommend that you split your 390,000 records into manageable transactions:

let jsonArray: [NSDictionary] = // ... 390,000 records
let chunkSize = 1000
for i in jsonArray.startIndex.stride(through: jsonArray.endIndex, by: chunkSize) {

    let chunkArray = jsonArray[i ..< i.advancedBy(chunkSize, limit: jsonArray.endIndex)]
    CoreStore.beginAsynchronous { (transaction) in

        // ... process chunkArray

        transaction.commit()
    }
}

[milanhorvatovic]

Did you try check when it's your NotificationObserver or NSManagedObjectContext objects are deallocated?

[JohnEstropia]

@milanhorvatovic You can confirm for yourself how they are deallocated.

NotificationObserver: The observer is deallocated when the transaction completes: https://github.com/JohnEstropia/CoreStore/blob/f486ace437958c7715a69074dc8f46c9f95df4cd/Sources/Internal/NotificationObserver.swift#L53 Note that the observerForWillSaveNotification closure does not capture any references from outside the block, so we can assume that no retain cycles can occur.)

NSManagedObjectContext: The context is retained only by the transaction, so after the transaction ends the context and all its objects' memory are released. See https://github.com/JohnEstropia/CoreStore/blob/f486ace437958c7715a69074dc8f46c9f95df4cd/Sources/Transactions/DataStack%2BTransaction.swift#L44 Notice how the AsynchronousDataTransaction is created and not assigned anywhere. Once the perform() task ends the transaction goes away along with the context.

[milanhorvatovic]

Sorry but you are wrong, because your deinit from NotificationObserver will never be called. Your idea about NotificatioObserver is ok, but problem in this situation is with strong reference to context holded by observer instance.

You are creating context instance for all types of your transactions in BaseDataTransaction.swift by let context = mainContext.temporaryContextInTransactionWithConcurrencyType( queue == .Main ? .MainQueueConcurrencyType : .PrivateQueueConcurrencyType ) so that is same for all transactions. Inside that extension for NSManagedObjectContext you are constructing everything necessary. Also here is registering for notification about saving context context.setupForCoreStoreWithContextName("com.corestore.temporarycontext"). In this function is registration for notification via your NotificationObserver self.observerForWillSaveNotification = NotificationObserver( ... ) and here is problem.

Your computed variable is as strong reference to observer. That is ok and it's necessary: private var observerForWillSaveNotification: NotificationObserver?. Complication is in NotificationObserver because he is also holding strong reference to context let object: AnyObject?

Everything you can check yourself by adding simple line print("\(#file) \(#line)") into deinit { ... } of NotificationObserver. Actually that destruction function will not be called. If you exchange let object: AnyObject? into weak var object: AnyObject? it will be called. I don't know what and how you plan your architecture, so i can't say that is bullet proof solution.

[JohnEstropia]

@milanhorvatovic Good find there, thanks! I'll reopen the ticket and check this later today

[JohnEstropia]

I investigated why our tests missed this, it turns out Instruments doesn't consider this a leak 😭 Will push a fix by the end of the day

[JohnEstropia]

@milanhorvatovic Apologies for misunderstanding your initial problem. Fixed the leak in v2.0.4, hope it clears the problem for you.

[milanhorvatovic]

thank you for quick fix, now it works fine. that issue is hard to find via Instruments, because objects are still valid, but unreferenced except each one together. Standard situation about leak is in memory exists object(s) with retained count but it's unowned.