-
Notifications
You must be signed in to change notification settings - Fork 956
Apple friendly way to madvice on commit/decommit: MADV_FREE_REUSABLE/MADV_FREE_REUSE #1097
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
base: dev
Are you sure you want to change the base?
Conversation
src/prim/unix/prim.c
Outdated
// if this fails - fallback to MADV_DONTNEED | ||
#endif | ||
|
||
if (err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On non-Apple platforms, err
is still 0 here, right? Or am I misreading?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, you're right! I made a mistake, sorry!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed, could you please look again?
Thanks for the PR and putting attenting to the For |
So far current code in With
I added
Thanks!! Could you please explain what
Will let you know soon if it is worth it - will have actual numbers on a big population |
Thanks @Jura-Z for the reply. I am surprised that However, it seems that By default we use |
I hesitantly added initial support for these flags; it is active in a non-debug build or if (Also, if it seems better, it would be great if you could compare the latest |
Hi @daanx, I think we need to add a couple of --- a/src/prim/unix/prim.c
+++ b/src/prim/unix/prim.c
@@ -503,8 +503,9 @@ int _mi_prim_commit(void* start, size_t size, bool* is_zero) {
int _mi_prim_reuse(void* start, size_t size) {
#if defined(__APPLE__) && defined(MADV_FREE_REUSE)
return unix_madvise(start, size, MADV_FREE_REUSE);
- #endif
+ #else
MI_UNUSED(start); MI_UNUSED(size);
+ #endif
return 0;
} Otherwise it does not compile on Linux. |
We still early, but so far looks like with this PR we have completely different OOM picture. Thanks a lot for your change, the only thing I wanted to point out - Google Chrome is the main source of knowledge about |
Thanks -- very interesting. On my initial tests it seems a bit slower than MADV_DONTNEED though (but it may be worth it if it fixes the memory accounting). I am mostly hesitant due to lack of understanding/documentation what it does exactly ... :-) |
…ble use of MADV_FREE_REUSE on a reset (issue #1097)
It would be really interesting to talk more, maybe share some other things we found in terms of mimalloc v2 vs v3. I sent you an email to the only one email I could find. If you have any way to contact - me and @Noxybot could share more insights if you're interested |
MADV_FREE_REUSABLE
/MADV_FREE_REUSE
on decommit/commit helps significantly with memory consumption, leading to much less OOM crashes on iOS and macOS.I'm pretty sure this will resolve #1025
For more information about Darwin calls check: