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

Add an option to use the mimalloc memory allocator #170

Merged
merged 1 commit into from
Apr 5, 2023
Merged

Conversation

PeterTh
Copy link
Contributor

@PeterTh PeterTh commented Mar 30, 2023

Mimalloc is included as a submodule, used with FetchContent (as per convention).
Increases the required CMake version to 3.21 to enable $<TARGET_OBJECTS:.

MiMalloc generally seems to improve overall performance in the more realistic/larger microbenchmark scenarios by ~30% compared to the default allocator on GPUC2, which is quite significant for a simple allocator change:

It does not decrease performance in any microbenchmark (log-y):

Drive-by:

  • add a few common entries to gitignore
  • enable compile_commands.json generation by default (we have discussed that we should do this in several F2F meetings before)

@PeterTh PeterTh requested review from psalz and fknorr March 30, 2023 12:14
@BlackMark29A
Copy link

Off-topic, but since you are already drive-by-ing some stuff and are touching the main cmake file, can you also drive-by fix the chooosing typo in the main cmake file in line 68?

@PeterTh
Copy link
Contributor Author

PeterTh commented Mar 30, 2023

We've had an offline discussion regarding the bump in CMake min version. I'll look into whether we can link dynamically to avoid that, and what the perf impact (if any) is.

@PeterTh PeterTh force-pushed the mimalloc branch 2 times, most recently from d96d6cc to 2f67f13 Compare March 30, 2023 14:03
@PeterTh
Copy link
Contributor Author

PeterTh commented Mar 30, 2023

The PR has been modified to use mimalloc as a shared library.

This has two consequences:

  • it does not require us to bump the CMake version requirement,
  • but only allows us to easily override new/delete-based allocations. However, we don't do C-style allocations in Celerity.

Benchmarks are slightly different (both in positive and negative direction), but the overall picture and advantage compared to the default allocator remains the same.

Copy link
Contributor

@fknorr fknorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, LGTM up to some minor details!

CMakeLists.txt Outdated Show resolved Hide resolved
src/runtime.cc Outdated Show resolved Hide resolved
mimalloc is included as a submodule.
Also add a few common entries to gitignore,
and enable compile_commands generation by default.
Copy link
Member

@psalz psalz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@PeterTh PeterTh merged commit 234e3d2 into master Apr 5, 2023
@PeterTh PeterTh deleted the mimalloc branch April 5, 2023 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants