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

supports mimalloc for ARC/ORC (1.15x ~ 1.22x speedup booting the compiler) #20359

Closed
wants to merge 14 commits into from

Conversation

ringabout
Copy link
Member

@ringabout ringabout commented Sep 15, 2022

Credits to @Yardanico
ref https://github.com/Yardanico/mimalloc_nim

I added the commit by
git commit --amend --author="Danil Yarantsev tiberiumk12@gmail.com" -m "supports mimalloc for ARC/ORC"

@ringabout
Copy link
Member Author

ringabout commented Sep 15, 2022

The first step is to check whether it reduces booting compilation time. I will refactor it later. Making it work comes first.

@ringabout
Copy link
Member Author

ringabout commented Sep 15, 2022

It is weird that importing std/os in the global Nim config increases 200 MiB memory usage.

@ringabout
Copy link
Member Author

ringabout commented Sep 15, 2022

The booting time seems to drop at some extent by testing locally and on the CI

ubuntu-20.04 (batch: 0_3) 15.347s => 12.546s (1.22x)
ubuntu-20.04 (batch: 1_3) 9.140s => 7.867s (1.16x)
ubuntu-20.04 (batch: 2_3) 12.374s => 10.688s (1.15x)

@ringabout ringabout changed the title supports mimalloc for ARC/ORC supports mimalloc for ARC/ORC (1.15x ~ 1.22x speedup booting the compiler) Sep 15, 2022
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I think it's still beneficial to support dynamic mimalloc, because if you have multiple Nim DLLs, or the main Nim program and other Nim modules as shared libraries, if each of them has their own copy of mimalloc, you won't be able to share GC'd objects between the instances. So, similar to how nimrtl has it, we can just allow users to use dynamic mimalloc

@planetis-m

This comment was marked as resolved.

@ringabout
Copy link
Member Author

There might be a problem with diagnostic tools like heaptrack and sanitizers that work best with -d:useMalloc. As I understand this switch is now abused to mean mimalloc. Does this PR still allow them to work reliably? Have you tested?

Not really, I add a new --d:useMimAlloc switch. -d:useMalloc still works.

koch.nim Outdated Show resolved Hide resolved
@ringabout
Copy link
Member Author

Waiting for #19972

@Araq
Copy link
Member

Araq commented Sep 27, 2022

This is critical for the v2.0 release btw.

@ringabout
Copy link
Member Author

Sure.

@ringabout ringabout mentioned this pull request Sep 27, 2022
33 tasks
@Araq
Copy link
Member

Araq commented Dec 8, 2022

Closing for the time being as our allocator is also now threadsafe.

@Araq Araq closed this Dec 8, 2022
@ringabout ringabout deleted the pr_mimalloc branch December 8, 2022 15:00
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.

3 participants