-
Notifications
You must be signed in to change notification settings - Fork 883
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
chore: pull helio and adjust mimalloc for 32MiB segments #3174
base: main
Are you sure you want to change the base?
Conversation
kostasrim
commented
Jun 13, 2024
•
edited
Loading
edited
- pull helio
- use mimalloc 32MiB segments
- adjust SegmentAllocator
src/core/segment_allocator.cc
Outdated
static_assert(kSegLogSpan == kSegmentShift); | ||
static_assert((~kSegmentAlignMask) == (MI_SEGMENT_SIZE - 1)); | ||
// static_assert((~kSegmentAlignMask) == (MI_SEGMENT_SIZE - 1)); |
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.
99% Sure this was a mistake. When this was introduced it would not pass the assertion on the previous versions.
Also mimalloc uses 32MiB segments but we fetch 4MiB. We can bump this up to 8MiB
or 32MiB
to give 1-1 mapping.
I am not very familiar with SmallString
which uses this extensively so I will update on this.
@@ -17,16 +17,19 @@ namespace dfly { | |||
|
|||
/** | |||
* @brief Tightly coupled with mi_malloc 2.x implementation. | |||
* Fetches 4MiB segment pointers from the allocated pointers. | |||
* Fetches 32MiB segment pointers from the allocated pointers. |
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.
It doesn't really make a difference if we use 8MiB
here as it was before we switched to mimalloc version that used 4MiBSegments
because either way SmallString
will hold a 4 bytes
pointer so it won't affect its memory footprint which is the goal of this anyway
passes locally we need to merge: romange/helio#283 and pull this here |