Skip to content

Pass size hint to jemalloc for faster deallocation#15071

Merged
tezc merged 2 commits into
redis:unstablefrom
tezc:zfree-with-size
May 8, 2026
Merged

Pass size hint to jemalloc for faster deallocation#15071
tezc merged 2 commits into
redis:unstablefrom
tezc:zfree-with-size

Conversation

@tezc

@tezc tezc commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

This PR is based on valkey-io/valkey#453 and valkey-io/valkey#694

When jemalloc frees memory, it performs a lookup to find the allocation's size class. sdallocx() lets us skip this lookup by passing the size we already know. Introduced a new free function wrapper for this: zfree_with_size().
Note: Impact of this optimization is only visible on hot paths e.g. on repeated memory deallocations.

For the initial phase, I integrated this at sdsfree() only. Over time, we may expand the usage of this new API for other performance sensitive paths.

For testing, added jemalloc config --enable-opt-size-checks to the daily fortify build. This makes jemalloc validate that the size passed to sdallocx() matches the actual allocation's size class, aborting on mismatch.


Signed-off-by: Vadym Khoptynets [email protected]
Signed-off-by: Madelyn Olson [email protected]
Co-authored-by: Madelyn Olson [email protected]
Signed-off-by: ranshid [email protected]


Note

Medium Risk
Touches low-level memory deallocation paths (zmalloc/sdsfree) and introduces sized frees, where an incorrect size hint could crash under jemalloc or skew memory accounting (mitigated by added jemalloc size-check CI coverage).

Overview
Speeds up jemalloc deallocation by adding zfree_with_size() (wrapping je_sdallocx() with a usable-size hint) and wiring it through SDS via a new s_free_with_size macro.

Updates sdsfree() to use sized free for all SDS types except SDS_TYPE_5 (which lacks reliable allocation metadata), and removes the unused sdsfreeusable() API.

Strengthens test coverage by enabling jemalloc --enable-opt-size-checks in the daily fortify workflow and treating jemalloc "size mismatch" output as a sanitizer failure in the test harness.

Reviewed by Cursor Bugbot for commit 22168fe. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 2b05c14. Configure here.

Comment thread src/sds.h Outdated
@augmentcode

augmentcode Bot commented Apr 18, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Adds a sized-free API to speed up jemalloc deallocation by passing a size hint to sdallocx() and avoiding size-class lookup on hot free paths.

Changes:

  • Introduce zfree_with_size() (jemalloc: je_sdallocx) and expose it to SDS via s_free_with_size
  • Use sized-free for most sdsfree() calls (non-SDS_TYPE_5) and add a fast path for freeing non-kvobj EMBSTR string objects
  • Remove the now-unneeded sdsfreeusable() API
  • Harden testing by enabling jemalloc --enable-opt-size-checks in the daily fortify build and flagging jemalloc “size mismatch” logs as test failures

Technical Notes: Correctness depends on callers passing a size that matches the allocation’s size class; CI now validates this via jemalloc’s opt size checks.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread src/sds.h Outdated
Comment thread src/zmalloc.c
@tezc

tezc commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator Author

augment review

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@tezc tezc added the action:run-benchmark Triggers the benchmark suite for this Pull Request label Apr 18, 2026
@fcostaoliveira

fcostaoliveira commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

CE Performance Automation : step 1 of 2 (build) DONE.

This comment was automatically generated given a benchmark was triggered.
Started building at 2026-05-07 13:49:46.956455 and took 59 seconds.
You can check each build/benchmark progress in grafana:

  • git hash: 22168fe
  • git branch: tezc:zfree-with-size
  • commit date and time: n/a
  • commit summary: n/a
  • test filters:
    • command priority lower limit: 0
    • command priority upper limit: 10000
    • test name regex: .*
    • command group regex: .*

You can check a comparison in detail via the grafana link

@fcostaoliveira

fcostaoliveira commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

CE Performance Automation : step 2 of 2 (benchmark) RUNNING...

This comment was automatically generated given a benchmark was triggered.

Started benchmark suite at 2026-04-18 19:27:34.388513 and took 168.772082 seconds up until now.
Status: [--------------------------------------------------------------------------------] 0.3% completed.

In total will run 331 benchmarks.
- 330 pending.
- 1 completed:
- 1 successful.
- 0 failed.
You can check a the status in detail via the grafana link

@sundb

sundb commented Apr 20, 2026

Copy link
Copy Markdown
Collaborator

what zfree_with_size want to save is zmalloc_size(), but we already have free_with_usize().

@tezc

tezc commented Apr 20, 2026

Copy link
Copy Markdown
Collaborator Author

@sundb it is faster "in jemalloc". Avoids emap/rtree lookup:

Screenshot from 2026-04-20 08-51-25
Screenshot from 2026-04-20 08-52-26

@sundb

sundb commented Apr 20, 2026

Copy link
Copy Markdown
Collaborator

@sundb it is faster "in jemalloc". Avoids emap/rtree lookup:

@tezc great, let's wait for the benchmark result.

@tezc tezc force-pushed the zfree-with-size branch from f9ca64a to 30f9b26 Compare April 20, 2026 06:34
@tezc tezc added action:run-benchmark Triggers the benchmark suite for this Pull Request and removed action:run-benchmark Triggers the benchmark suite for this Pull Request labels Apr 20, 2026
@tezc tezc merged commit 7cf6363 into redis:unstable May 8, 2026
17 of 18 checks passed
@sundb sundb removed the action:run-benchmark Triggers the benchmark suite for this Pull Request label May 8, 2026
tezc added a commit that referenced this pull request May 9, 2026
While profiling command execution, I noticed that command argv object
alloc/free overhead is quite high for workloads with many small
arguments (e.g. `HSET` with many fields). The effect is much more
visible with pipelining when Redis becomes CPU bound.

I experimented with replacing argv object alloc/free with a simple
object pool and saw significant speedups.
(Note: related effort around this topic:
#13726)

In this PR, I tried to improve the main hotspots in the memory
allocation path (focusing on command arg allocations) to close the gap
with custom pool performance, so we can avoid having a dedicated memory
pools and let the whole codebase benefit from these optimizations.

## Changes

### 1) Faster dealloc via passing size hint to jemalloc (separate PR
#15071)
Jemalloc does more work than an object pool on free (a lookup on a tree
to find the allocation's size class). For some deallocations, we can
reduce free path overhead by passing a size hint to jemalloc (i.e.
`sdallocx()`) which can skip metadata lookup in the common case. This PR
introduces `zfree_with_size()` and uses it where we can know the
allocation size i.e. `OBJ_ENCODING_EMBSTR` objects in `decrRefCount()`
and SDS free path.

### 2) Reduce atomic operation cost for stat updates
`update_zmalloc_stat_alloc()` / `update_zmalloc_stat_free()` previously
used atomic read-modify-write (RMW) operations (`atomicIncrGet` /
`atomicDecr`) which can emit expensive locked instructions on x86.

When we can guarantee a single writer to a counter, we can use a cheaper
load+add+store sequence instead of a locked RMW. This PR gives the first
16 threads dedicated slots for used_memory stats (intended to cover the
main thread/ I/O threads) so they can use this single writer fast path.
Threads beyond that fall back to a shared pool and continue to use full
atomic RMW.

### 3) Improve jemalloc tcache hit rate 

With the default `lookahead=16` config, a pipelined HSET with ~20 fields
does ~40 small allocations per command (fields + values), so you can get
16 x 40 = ~640 allocations. When args are small, many of these land in
the 32 byte size class (often `EMBSTR`). Jemalloc’s default per-bin
tcache cap is 200, so this kind of burst overflows the cache and it does
frequent flushes. I raised the small-bin tcache limits
(lg_tcache_nslots_mul:3, tcache_nslots_small_max:1000) to handle these
bursts better. In the worst case, tcache may have a higher memory usage
due to this change. Perhaps, another option was lowering `lookahead` to
tune it differently.

### 4) Inlining
When you have a simple pool, it has a few small functions and it is easy
for compiler to inline them. Compared to that, jemalloc alloc/free path
has a deeper call stack. Also, jemalloc was not compiled with `-flto`
which was preventing inlining jemalloc functions. As part of this PR, I
added `-flto` flag to jemalloc when it is enabled for Redis.

Compiler also chooses not to inline some hot path functions in Redis.
This suggests PGO (profile-guided optimization) could provide additional
wins and perhaps we can start experimenting with it sometime. We could
try to force inlining with attributes like `always_inline` but it is
hard to apply across a deep call stack and misuse can cause code bloat.
So, rather than going in this direction, I added `inline` keyword to
some functions for now. This doesn't make compiler to inline all hot
path functions but at least it is a step ahead. (If we can further
improve this in future, performance gets very close to custom memory
pool implementation).

## Benchmark results

Commands were like:

```
memtier_benchmark   --command="HSET __key__ username john_doe email [email protected] password hashed_pwd_123 created_at 1709125200 updated_at 1709125200 first_name John last_name Doe phone_number +1234567890 address 123_Main_St city NewYork country USA postal_code 10001 company Acme_Corp job_title Engineer bio Loves_coding"   --command-ratio=1   --command-key-pattern=P   --key-prefix="hsetkey"   --key-minimum=1   --key-maximum=100000   -n 1000000   -c 50   -t 2   --hide-histogram --pipeline 50
```

| Benchmark | Improvement |
| --- | ---: |
| SET | +0% |
| SET (pipeline) | +8% |
| HSET 15 fields | +2% |
| HSET 15 fields (pipeline) | +17% |
| ZADD 15 elements| +3% |
| ZADD 15 elements (pipeline) | +15% |
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