Skip to content
This repository has been archived by the owner on Aug 10, 2021. It is now read-only.

Mimalloc allocator integration #3704

Merged
merged 3 commits into from Dec 27, 2019
Merged

Mimalloc allocator integration #3704

merged 3 commits into from Dec 27, 2019

Conversation

LepilkinaElena
Copy link
Contributor

No description provided.

runtime/build.gradle Outdated Show resolved Hide resolved
runtime/build.gradle Outdated Show resolved Hide resolved
this == KonanTarget.IOS_X64

fun KonanTarget.supportsOptimizedAllocator(): Boolean =
Copy link
Contributor

Choose a reason for hiding this comment

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

supportsOptimizedAllocator is a bit unclear. Maybe supportsMimallocAllocatior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to connect this with exact allocator, there is possibility that we change or add another one.

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is that the set of supported targets is determined by allocator itself, not target. The set for jemalloc or any other allocator may be different.
Another possible solution is to add enum of alternative allocators and its entry to supportsOptimizedAllocator.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another good thing is to describe why other targets are not supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's a good idea with enum, but now it would be strange to have enumeration with one allocator. I don't want to show name of allocator in compiler options. Now we have just std and opt. It seems that same names everywhere are better.

val (executable, defaultFlags, srcFilesPatterns) =
if (language == Language.C)
Triple("clang",
commonFlags + listOf("-std=gnu11", "-O3", "-Wall", "-Wextra", "-Wno-unknown-pragmas",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why -O3 for C and -O2 for C++? Also why there is no -fPIC here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to save options that are set in build scripts for allocator, it's already tuned. I thought about -fPIC, and I understood it wasn't used because of some assembly parts for some platforms. Assembly code isn't written in proper way for position independent code.
If you see real problems with this I need to turn on and test again all platforms it influences.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be easier to update versions if we save settings, because there are some hacks to speed up allocator.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried to save options that are set in build scripts for allocator, it's already tuned.

Then it is better to mention it with a comment.

If you see real problems with this I need to turn on and test again all platforms it influences.

There was an Android target that wasn't compiling because of -fPIC absence, but I don't remember which one :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I built some projects with native Android, if we meet a problem then will be a time to decide. I think without exact use case it's better to save default options set.

@@ -317,8 +317,8 @@ def createTestTasks(File testRoot, Class<Task> taskType, Closure taskConfigurati

void dependsOnPlatformLibs(Task t) {
if (!useCustomDist) {
def testTarget = project.testTarget
if (testTarget != null && testTarget != project.hostName) {
def testTarget = project.target
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was for running raspberry-pi tests. @PavelPunegov should commit sane changes on master.

if (configuration.get(KonanConfigKeys.ALLOCATION_MODE) == "mimalloc") {
if (!target.supportsMimallocAllocator()) {
configuration.report(CompilerMessageSeverity.STRONG_WARNING,
"Optimized allocation mode isn't supported on target ${target.name}. Used standard mode.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Optimized allocation mode -> "mimalloc allocator".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants