Memory termination #476
Memory termination #476
Conversation
|
I'm really impressed by the clever way you've used memory-mapped files! It's a great solution. I just wanted to check in about something. How can we be sure that data cached in memory is going to be written to disc by the kernel in case of a crash? |
The way I understand it is that mmap is handled in kernel space and not user space, which leads to file based mmap to basically be a “kernel cache” that is always dumped to disk based on the mapping sync functions, and a process being terminated is one of those instances where the kernel will explicitly call those sync functions, which leads to crash resilience. |
|
@GLinnik21 Can you reopen the PR so I can change the base to main and we can continue work on it? |
|
Oh sorry. It got automatically closed because I merged and deleted |
|
I wasn't able to change the base for some reason, likely because the PR is closed. |
|
Oh, there we go :) thank you. |
|
Could you add some tests to cover the new functionality where possible? It would help maintain the quality and stability of the project. Thanks! |
I've added a few tests. If there's anything in particular you'd like to be tested feel free to point it out, I'm happy to add more. |
|
Any clues what’s going on with this check, I don’t see what’s wrong. |
Let's try to rerun for now. |
|
I wanted to share a thought on organizing our code. When we put multiple class implementations in a single file, especially in both implementation and header files, it can make the code harder to read and maintain. Keeping everything in separate files can really simplify code reading and understanding. Some of our files with multiple implementations have grown to several hundred lines of code, which can be overwhelming. What do you think about splitting them up to make things more manageable? |
Update the context to have info about if we're currently in a crash handler.
We might want ot think of adding Mac Catalyst to KSCRASH_HAS_UIAPPLICATION.
enabled fatal reporting as well.
naftaly
force-pushed
the
Memory-Termination
branch
from
e82109b
to
4969928
Compare
|
@bamx23 @GLinnik21 I addressed the last few pieces of feedback. I found a simple way to enable/disable reporting OOMs but keep the data flowing and added to other types of reports. I also wanted to let both of you know I wrote a small piece about what we did here. https://medium.com/@alexandercohen/reducing-memory-terminations-in-ios-apps-3e76797ca5bd |
|
Could you folks rerun the actions, they seem to have failed due to a timeout. |
As always, the more tests, the better. If other code is too low-level to test, I'm fine with that. |
Wow! I read the whole article and found it very insightful. Thank you for the shoutout! |
Adding OOM support to KSCrash.
In a gist, this works by writing an OOM report to disk outside of the report area for later modification. We also keep track of memory information through a memory mapped file for crash resistance, we write to it anytime memory limit or pressure changes. On termination, that data is basically a breadcrumb we can use on cold start.
On cold start, we look for that mapped file, if memory pressure is >= critical or memory level is >= critical it means we were terminated due to memory. At this point we'll read in the OOM report we write to disk on the previous app launch, modify it to fit our needs with latest information and move it to the report folder where the rest of the system will pick it up and send it in.
One thing that would make everything better is to have new repositories for each cold start. it can make handling files a lot more robust.