Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Use mimalloc as the default allocator #602

Merged
merged 5 commits into from
Aug 17, 2022
Merged

Use mimalloc as the default allocator #602

merged 5 commits into from
Aug 17, 2022

Conversation

loiclec
Copy link
Contributor

@loiclec loiclec commented Aug 10, 2022

What does this PR do?

Use mimalloc as the global allocator for milli's benchmarks on macOS.

Why?

On Linux, we use jemalloc, which is a very fast allocator. But on macOS, we currently use the system allocator, which is very slow. In practice, this difference in allocator speed means that it is difficult to gain insight into milli's performance by running benchmarks locally on the Mac.

By using mimalloc, which is another excellent allocator, we reduce the speed difference between the two platforms.

@loiclec loiclec added benchmarks Related to the benchmarks no breaking The related changes are not breaking (DB nor API) labels Aug 10, 2022
@loiclec loiclec requested a review from Kerollmops August 10, 2022 14:00
@irevoire
Copy link
Member

From what I see mimalloc is almost always faster than jemalloc, why not use it for linux as well?

Copy link
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

Indeed, why not using either jemalloc or mimalloc on all platforms. And if we use mimalloc it could probably also work on Windows too (as it is made by Microsoft)?

benchmarks/Cargo.toml Show resolved Hide resolved
@loiclec
Copy link
Contributor Author

loiclec commented Aug 16, 2022

It looks like mimalloc is indeed faster than jemalloc, so I have enabled it on all platforms now.

group                                                                     indexing_main_950d8e4c                 indexing_mimalloc-benchmarks_fb2b6c0c
-----                                                                     ----------------------                 -------------------------------------
indexing/-geo-delete-facetedNumber-facetedGeo-searchable-                 1.00     35.6±2.96ms        ? ?/sec    1.09     38.7±4.25ms        ? ?/sec
indexing/-movies-delete-facetedString-facetedNumber-searchable-           1.00      8.3±2.72ms        ? ?/sec    1.33     11.1±5.47ms        ? ?/sec
indexing/-movies-delete-facetedString-facetedNumber-searchable-nested-    1.05     12.7±2.56ms        ? ?/sec    1.00     12.1±1.57ms        ? ?/sec
indexing/-songs-delete-facetedString-facetedNumber-searchable-            1.01     42.7±6.45ms        ? ?/sec    1.00     42.3±4.80ms        ? ?/sec
indexing/-wiki-delete-searchable-                                         1.05   283.0±18.79ms        ? ?/sec    1.00   268.9±22.20ms        ? ?/sec
indexing/Indexing geo_point                                               1.07      66.0±0.40s        ? ?/sec    1.00      61.9±0.65s        ? ?/sec
indexing/Indexing movies in three batches                                 1.07      16.7±0.18s        ? ?/sec    1.00      15.6±0.09s        ? ?/sec
indexing/Indexing movies with default settings                            1.07      14.0±0.30s        ? ?/sec    1.00      13.0±0.27s        ? ?/sec
indexing/Indexing nested movies with default settings                     1.05      10.8±0.18s        ? ?/sec    1.00      10.3±0.31s        ? ?/sec
indexing/Indexing nested movies without any facets                        1.06       9.6±0.11s        ? ?/sec    1.00       9.0±0.09s        ? ?/sec
indexing/Indexing songs in three batches with default settings            1.06      67.4±1.16s        ? ?/sec    1.00      63.5±0.61s        ? ?/sec
indexing/Indexing songs with default settings                             1.08      59.9±1.21s        ? ?/sec    1.00      55.7±0.58s        ? ?/sec
indexing/Indexing songs without any facets                                1.07      54.4±1.01s        ? ?/sec    1.00      50.9±0.58s        ? ?/sec
indexing/Indexing songs without faceted numbers                           1.06      58.9±1.21s        ? ?/sec    1.00      55.4±1.23s        ? ?/sec
indexing/Indexing wiki                                                    1.05   1077.7±25.72s        ? ?/sec    1.00   1022.7±11.20s        ? ?/sec
indexing/Indexing wiki in three batches                                   1.04    1193.7±7.22s        ? ?/sec    1.00    1143.3±4.03s        ? ?/sec
indexing/Reindexing geo_point                                             1.04      25.6±0.08s        ? ?/sec    1.00      24.5±0.14s        ? ?/sec
indexing/Reindexing movies with default settings                          1.00    335.8±9.11ms        ? ?/sec    1.00   334.6±29.84ms        ? ?/sec
indexing/Reindexing songs with default settings                           1.08       7.2±0.14s        ? ?/sec    1.00       6.7±0.08s        ? ?/sec
indexing/Reindexing wiki                                                  1.03   1750.9±28.38s        ? ?/sec    1.00   1693.3±29.61s        ? ?/sec

Copy link
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

I am wondering why we are only using mimalloc for the benchmarks and not everything.

@Kerollmops Kerollmops changed the title Use mimalloc for benchmarks on macOS Use mimalloc as the default allocator Aug 16, 2022
helpers/Cargo.toml Outdated Show resolved Hide resolved
@loiclec
Copy link
Contributor Author

loiclec commented Aug 17, 2022

mimalloc is used for everything now. Well, almost. It could also be used as a dev-dependency for milli's tests I guess. But I don't think it is very important, and I am afraid of having conflicting global allocators somehow.

Copy link
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

Could you, please, sort the Cargo.toml dependencies?

helpers/Cargo.toml Outdated Show resolved Hide resolved
@loiclec
Copy link
Contributor Author

loiclec commented Aug 17, 2022

Yes, done :-)

Copy link
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

Thank you very much!
bors merge

@bors
Copy link
Contributor

bors bot commented Aug 17, 2022

@bors bors bot merged commit cd2635c into main Aug 17, 2022
@bors bors bot deleted the mimalloc-benchmarks branch August 17, 2022 10:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
benchmarks Related to the benchmarks no breaking The related changes are not breaking (DB nor API)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants