Skip to content

Add zmalloc_aligned() and fix SPMC queue buffer alignment#3504

Merged
madolson merged 5 commits into
valkey-io:unstablefrom
sarthakaggarwal97:daily-fixes-20260414
Apr 23, 2026
Merged

Add zmalloc_aligned() and fix SPMC queue buffer alignment#3504
madolson merged 5 commits into
valkey-io:unstablefrom
sarthakaggarwal97:daily-fixes-20260414

Conversation

@sarthakaggarwal97

@sarthakaggarwal97 sarthakaggarwal97 commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

The SPMC queue from #3324 needs each spmcCell to be cache-line aligned, but plain zmalloc() does not guarantee that in all build configurations.

This change introduces zmalloc_cache_aligned() and uses it for the SPMC queue buffer allocation in spmcInit().

Failing CI: https://github.com/valkey-io/valkey/actions/runs/24374139344

Worked with Codex!

@sarthakaggarwal97 sarthakaggarwal97 marked this pull request as draft April 14, 2026 16:51
@sarthakaggarwal97 sarthakaggarwal97 marked this pull request as ready for review April 14, 2026 17:03
@sarthakaggarwal97 sarthakaggarwal97 added the run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) label Apr 14, 2026
@codecov

codecov Bot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.42%. Comparing base (03c2d4c) to head (86961f1).
⚠️ Report is 6 commits behind head on unstable.

Files with missing lines Patch % Lines
src/zmalloc.c 80.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #3504      +/-   ##
============================================
+ Coverage     76.21%   76.42%   +0.21%     
============================================
  Files           159      159              
  Lines         81672    81695      +23     
============================================
+ Hits          62243    62439     +196     
+ Misses        19429    19256     -173     
Files with missing lines Coverage Δ
src/queues.c 99.35% <100.00%> (ø)
src/unit/test_queues.cpp 99.57% <100.00%> (+<0.01%) ⬆️
src/unit/test_zmalloc.cpp 100.00% <100.00%> (ø)
src/zmalloc.c 84.15% <80.00%> (-0.17%) ⬇️

... and 24 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sarthakaggarwal97 sarthakaggarwal97 marked this pull request as draft April 14, 2026 17:49
@sarthakaggarwal97 sarthakaggarwal97 marked this pull request as ready for review April 14, 2026 19:07
@sarthakaggarwal97

Copy link
Copy Markdown
Contributor Author

Macos Test should get stabilised by #3509

@sarthakaggarwal97 sarthakaggarwal97 changed the title Fix SPMC queue buffer allocation to guarantee spmcCell is cache-line aligned Add zmalloc_aligned() and fix SPMC queue buffer alignment Apr 16, 2026
@sarthakaggarwal97 sarthakaggarwal97 force-pushed the daily-fixes-20260414 branch 2 times, most recently from 20e266c to 83bf327 Compare April 17, 2026 03:08
Comment thread src/zmalloc.h Outdated
Comment thread src/unit/test_zmalloc.cpp Outdated
Introduce zmalloc_aligned(alignment, size) for cache-line-aligned heap
allocations with full Valkey memory tracking. On HAVE_MALLOC_SIZE platforms
it wraps posix_memalign directly. On !HAVE_MALLOC_SIZE platforms it
over-allocates and stores the raw pointer and a flag-tagged size before
the aligned region so zfree() can transparently recover the original
pointer.

Use zmalloc_aligned() in spmcInit() to guarantee each spmcCell is
cache-line aligned, fixing UBSan failures in sanitizer CI builds
which force MALLOC=libc.

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Replace the general-purpose zmalloc_aligned(alignment, size) with a
simpler zmalloc_cache_aligned(size) that always aligns to CACHE_LINE_SIZE.

This is the only alignment any caller needs, so drop:
- The !HAVE_MALLOC_SIZE manual alignment path (MSB flag, raw pointer
  storage, modified zfree/zfree_with_size/zmalloc_size)
- The alignment validation helper
- The posix_memalign wrapper

The result is ~70 fewer lines with the same practical effect.
On !HAVE_MALLOC_SIZE platforms (none that Valkey ships on), it panics
at startup rather than silently corrupting memory.

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
@sarthakaggarwal97

Copy link
Copy Markdown
Contributor Author

The test failures are not related to this change. Related to #2936 and test-ubuntu-latest ones would be fixed by #3511

@madolson madolson merged commit 5abf79e into valkey-io:unstable Apr 23, 2026
63 of 66 checks passed
sarthakaggarwal97 added a commit to sarthakaggarwal97/valkey that referenced this pull request Apr 23, 2026
…3504)

The SPMC queue from valkey-io#3324 needs each `spmcCell` to be cache-line
aligned, but plain `zmalloc()` does not guarantee that in all build
configurations.

This change introduces `zmalloc_cache_aligned()` and uses it for the
SPMC queue buffer allocation in `spmcInit()`.

Failing CI: https://github.com/valkey-io/valkey/actions/runs/24374139344

---------

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
madolson pushed a commit that referenced this pull request Apr 27, 2026
The SPMC queue from #3324 needs each `spmcCell` to be cache-line
aligned, but plain `zmalloc()` does not guarantee that in all build
configurations.

This change introduces `zmalloc_cache_aligned()` and uses it for the
SPMC queue buffer allocation in `spmcInit()`.

Failing CI: https://github.com/valkey-io/valkey/actions/runs/24374139344

---------

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
sarthakaggarwal97 added a commit to sarthakaggarwal97/valkey that referenced this pull request Apr 27, 2026
…sers)

Removed valkey-io#3504, valkey-io#3545, valkey-io#3503 - these fix bugs introduced by valkey-io#3324, valkey-io#2936,

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
valkey-io#3392 respectively, all new in RC2. Users never experienced these bugs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants