Skip to content

Adds mimalloc as default feature.#10378

Merged
fdncred merged 1 commit intonushell:mainfrom
FilipAndersson245:main
Sep 15, 2023
Merged

Adds mimalloc as default feature.#10378
fdncred merged 1 commit intonushell:mainfrom
FilipAndersson245:main

Conversation

@FilipAndersson245
Copy link
Copy Markdown
Contributor

Description

This PR makes mimalloc the default allocator, it has been the default on the nightly builds of nushell now for a couple of months and the performance improvements are quite nice, measuring upwards of 30% faster startup time on Windows, and a bit smaller on Linux, #9415

User-Facing Changes

Tests + Formatting

After Submitting

@amtoine amtoine requested a review from fdncred September 15, 2023 16:54
@amtoine
Copy link
Copy Markdown
Member

amtoine commented Sep 15, 2023

@fdncred
i think you are our mimalloc expert, right? 😋

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Sep 15, 2023

@amtoine I'm not expert but I run nushell exclusively with extra,dataframe,mimalloc features on Windows, MacOS, and Ubuntu. I've had no issues whatsoever related to it. I'd be for making this default, but I have a suspicion that @jntrnr would be against it. JT said a while back that we should improve performance in other ways first. I could be wrong.

@ChrisDenton
Copy link
Copy Markdown
Contributor

ChrisDenton commented Sep 15, 2023

The default Windows allocator is slow but perhaps even more troubling is that all allocations are synchronized across threads. This can negate many of the perf advantages of multithreading unless you're very careful with every alloc/dealloc. Which is why I'd make the case for switching now. The patterns used to optimize around the Windows allocator may be very different to other allocators.

This is all imho, of course.

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Sep 15, 2023

Let's go!

@fdncred fdncred merged commit 9074015 into nushell:main Sep 15, 2023
hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

This PR makes `mimalloc` the default allocator, it has been the default
on the nightly builds of nushell now for a couple of months and the
performance improvements are quite nice, measuring upwards of 30% faster
startup time on Windows, and a bit smaller on Linux,
nushell#9415

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
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