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

Enable the jemalloc option --disable-cache-oblivious #1575

Merged
merged 7 commits into from
Jul 20, 2023

Conversation

aleksraiden
Copy link
Contributor

Inspired an Redis release notice, I add a two options for optimize jmalloc usages:

  • disable-tcache - Disable thread-specific caches for small objects. Objects are cached and released in bulk, thus reducing the total number of mutex operations.
  • disable-cache-oblivious - Disable cache-oblivious large allocation alignment for large allocation requests with no alignment constraints. If this feature is disabled, all large allocations are page-aligned as an implementation artifact, which can severely harm CPU cache utilization. For more info, please see a conversations in Redis - Set Jemalloc --disable-cache-oblivious to reduce memory overhead redis/redis#12315 where this options are available started at 7.3-rc2.

Disable thread-specific caches for small objects.  Objects are cached and released in bulk, thus reducing the total number of mutex operations.
Like in Redis 7.3 (see redis/redis#12315). 

Disable cache-oblivious large allocation alignment for large allocation requests with no alignment constraints.  If this feature is disabled, all large allocations are page-aligned as an implementation artifact, which can severely harm CPU cache utilization.
@mapleFU
Copy link
Member

mapleFU commented Jul 10, 2023

I think we should testing rather than copying the config directly from Redis. Redis store all parts in memory, and has too many small objects, like sds, skiplist node and so on. So small object would be a severe problem for it.
However, kvrocks stores these objects on a log-structure merge tree, and data is organized as sst/blocks. I guess there would be something different.

@aleksraiden
Copy link
Contributor Author

In my quick tests, these changes brought a slight improvement in terms of better freeing up memory when kvroks is idle.

@git-hulk
Copy link
Member

@aleksraiden Could you paste your test result here?

@aleksraiden
Copy link
Contributor Author

I haven't a clear env, but overall, if I run ./redis-benchmark -q -n 1000000 -P 10 -p 6666 my instance of kvrocks use around a 505 Mb RAM after a hour of idle (after a test are stopped). And, jmalloc-fix version use 432 Мб in this case too.

@git-hulk
Copy link
Member

@aleksraiden Thanks for your information. I feel good after looking through jemalloc/jemalloc#1098. Another question is why disable the tcache as well, did I miss anything?

@aleksraiden
Copy link
Contributor Author

@aleksraiden Thanks for your information. I feel good after looking through jemalloc/jemalloc#1098. Another question is why disable the tcache as well, did I miss anything?

As I think - because we don't need an a global cache for thread-specific objects, so if a small object usages only in one thread - this thread can feel free to manage this and release memory without use mutex for thread-safe operations.

@git-hulk
Copy link
Member

@aleksraiden Thanks for your information. I feel good after looking through jemalloc/jemalloc#1098. Another question is why disable the tcache as well, did I miss anything?

As I think - because we don't need an a global cache for thread-specific objects, so if a small object usages only in one thread - this thread can feel free to manage this and release memory without use mutex for thread-safe operations.

I prefer to keep it as it is unless we can prove that can bring benefits.

@aleksraiden
Copy link
Contributor Author

@aleksraiden Thanks for your information. I feel good after looking through jemalloc/jemalloc#1098. Another question is why disable the tcache as well, did I miss anything?

As I think - because we don't need an a global cache for thread-specific objects, so if a small object usages only in one thread - this thread can feel free to manage this and release memory without use mutex for thread-safe operations.

I prefer to keep it as it is unless we can prove that can bring benefits.

Redis use this options too, but if reviewers are OK, I can update PR and remove option about tcache

@git-hulk
Copy link
Member

git-hulk commented Jul 11, 2023

Redis use this options too

I didn't notice this option, would you mind giving me a link?

but if reviewers are OK, I can update PR and remove option about tcache

It's good for me to add if we can know/prove it did bring benefits.

@aleksraiden
Copy link
Contributor Author

Redis use this options too

I didn't notice, would you mind giving me a link?

but if reviewers are OK, I can update PR and remove option about tcache

It's good for me to add if we can know/prove it did bring benefits.

Oops, I'am SORRY, my mistake, just think about one options, but write for other, Yes, you are absolutely correct, Redis didn't use this options about tcache disabling.

@mapleFU
Copy link
Member

mapleFU commented Jul 11, 2023

  1. disable-tcache: I guess this might not benefits because at most of time, thread cache is an optimization
  2. as for disable-cache-oblivious, redis says it would only benefits 16kb allocations, however, I guess kvrocks could have many over 16kb allocation because of rocksdb.

Maybe we can take a look at TiKV's arguments: https://github.com/tikv/jemallocator/blob/main/jemalloc-ctl/src/opt.rs . It also uses RocksDB as storage engine.

@git-hulk
Copy link
Member

git-hulk commented Jul 11, 2023

Oops, I'am SORRY, my mistake, just think about one options, but write for other, Yes, you are absolutely correct, Redis didn't use this options about tcache disabling.

@aleksraiden Thanks for your investigation.

Maybe we can take a look at TiKV's arguments: https://github.com/tikv/jemallocator/blob/main/jemalloc-ctl/src/opt.rs . It also uses RocksDB as storage engine.

@mapleFU That's a good input.

Revert options --disable-tcache
git-hulk
git-hulk previously approved these changes Jul 11, 2023
@git-hulk git-hulk changed the title (deps) Jemalloc cache allocation options tune Enable the jemalloc option --disable-cache-oblivious Jul 11, 2023
@mapleFU
Copy link
Member

mapleFU commented Jul 11, 2023

Seems that by default, if we disable cache-oblivious, we will

  1. Not align the large chunk, which make 16kb page just uses 16kb of memory, rather than about 20kb, which brings more usable memory to fill the "block cache" required by system
  2. May harms some cache-obilivious. But seems that we don't have that

I guess maybe we can allow enable that for some read-heavy systems, that tent to fill the memory with block cache, so this is useful. Rocksdb might not have cache-oblivious algorithm, so it might not harms too much?

But I guess for most of workloads, they might have downgrade or not, because we have this https://github.com/facebook/rocksdb/blob/854eb76a8c8533779da7eaf21bef6266d9cca6aa/memory/arena.h#L32-L33 block size, it could become 16KB.

Maybe we can support a flag, and let different kind of user to specify their usage.

Another view is that Redis is a single-thread application, so most of time, it doesn't matters. If we care about the multi-core performance, it might harm the application.

add runtime options
mapleFU
mapleFU previously approved these changes Jul 20, 2023
@mapleFU
Copy link
Member

mapleFU commented Jul 20, 2023

Seems that CI compile failed :-)

Fix options for jemalloc
@aleksraiden
Copy link
Contributor Author

Updated. So, now we can set options in build, e.g. -D=DISABLE_CACHE_OBLIVIOUS=0|1 where 0 are default and only set to 1 add this options into jemalloc compile process.

@aleksraiden
Copy link
Contributor Author

Thanks @torwig for help with cmake

@git-hulk git-hulk requested a review from mapleFU July 20, 2023 09:56
@git-hulk
Copy link
Member

Thanks all, merging...

@git-hulk git-hulk merged commit 841f4e0 into apache:unstable Jul 20, 2023
p1u3o pushed a commit to p1u3o/incubator-kvrocks that referenced this pull request Aug 1, 2023
Inspired a Redis release notice, I add two options for optimizing jmalloc usages:

- disable-cache - Disable thread-specific caches for small objects.  Objects are cached and released in bulk, thus reducing the total number of mutex operations.
- disable-cache-oblivious - Disable cache-oblivious significant allocation alignment for large allocation requests with no alignment constraints.  If this feature is disabled, all large allocations are page-aligned as an implementation artifact, which can severely harm CPU cache utilization. For more info, please take a look at the conversations in Redis - redis/redis#12315 where these options are available starting at 7.3-rc2.
@aleksraiden aleksraiden deleted the jemalloc-build-options branch January 5, 2025 08:38
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