Skip to content

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

Open
sarthakaggarwal97 wants to merge 4 commits intovalkey-io:unstablefrom
sarthakaggarwal97:daily-fixes-20260414
Open

Add zmalloc_aligned() and fix SPMC queue buffer alignment#3504
sarthakaggarwal97 wants to merge 4 commits intovalkey-io:unstablefrom
sarthakaggarwal97:daily-fixes-20260414

Conversation

@sarthakaggarwal97
Copy link
Copy Markdown
Contributor

@sarthakaggarwal97 sarthakaggarwal97 commented Apr 14, 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

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
Copy link
Copy Markdown

codecov Bot commented Apr 14, 2026

Codecov Report

❌ Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.59%. Comparing base (6444717) to head (cbd4fb2).

Files with missing lines Patch % Lines
src/zmalloc.c 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           unstable    #3504   +/-   ##
=========================================
  Coverage     76.58%   76.59%           
=========================================
  Files           159      159           
  Lines         80019    80043   +24     
=========================================
+ Hits          61283    61307   +24     
  Misses        18736    18736           
Files with missing lines Coverage Δ
src/queues.c 99.32% <100.00%> (ø)
src/unit/test_queues.cpp 99.53% <100.00%> (+<0.01%) ⬆️
src/unit/test_zmalloc.cpp 100.00% <100.00%> (ø)
src/zmalloc.c 84.43% <80.00%> (-0.18%) ⬇️

... and 17 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

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