Prefer storing iterators on stack instead of the heap #14200
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
|
New Issues (1)Checkmarx found the following issues in this Pull Request
Fixed Issues (1)Great job! The following issues were fixed in this Pull Request
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors dictionary iteration throughout the Redis codebase to use stack-allocated iterators instead of heap-allocated ones. This optimization improves memory efficiency by replacing dictGetIterator()/dictSafeGetIterator() with dictInitIterator()/dictInitSafeIterator() and dictReleaseIterator() with dictResetIterator().
- Replace heap-allocated dictionary iterators with stack-allocated ones
- Update list iterator usage in redis-cli.c to use stack allocation pattern
- Convert dictionary iteration pattern consistently across all modules
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/t_zset.c | Convert sorted set dictionary iteration to stack allocation |
| src/t_set.c | Convert set dictionary iteration to stack allocation |
| src/t_hash.c | Convert hash dictionary iteration to stack allocation |
| src/sort.c | Convert sort operation dictionary iteration to stack allocation |
| src/server.c | Convert server command handling dictionary iterations to stack allocation |
| src/sentinel.c | Convert sentinel monitoring dictionary iterations to stack allocation |
| src/redis-cli.c | Convert redis-cli dictionary and list iterations to stack allocation |
| src/rdb.c | Convert RDB save operations dictionary iteration to stack allocation |
| src/pubsub.c | Convert pub/sub dictionary iterations to stack allocation |
| src/object.c | Convert object handling dictionary iterations to stack allocation |
| src/multi.c | Convert multi/transaction dictionary iterations to stack allocation |
| src/module.c | Convert module system dictionary iterations to stack allocation |
| src/latency.c | Convert latency tracking dictionary iterations to stack allocation |
| src/functions.c | Convert function system dictionary iterations to stack allocation |
| src/dict.h | Update dictForEach macro to use stack allocation pattern |
| src/defrag.c | Convert defragmentation dictionary iterations to stack allocation |
| src/debug.c | Convert debug functionality dictionary iterations to stack allocation |
| src/db.c | Convert database dictionary iterations to stack allocation |
| src/config.c | Convert configuration dictionary iterations to stack allocation |
| src/cluster_legacy.c | Convert legacy cluster dictionary iterations to stack allocation |
| src/cluster.c | Convert cluster dictionary iterations to stack allocation |
| src/blocked.c | Convert blocking operations dictionary iterations to stack allocation |
| src/aof.c | Convert AOF operations dictionary iterations to stack allocation |
| src/acl.c | Convert ACL system dictionary iterations to stack allocation |
Collaborator
Collaborator
|
@skaslev from the above data #14200 (comment), i'll trigger 2 extra runs for:
to be sure all data is solid. |
Collaborator
Automated performance analysis summaryThis comment was automatically generated given there is performance data available. In summary:
Improvements Table
Improvements test regexp names: memtier_benchmark-10Kkeys-load-hash-50-fields-with-10000B-values|memtier_benchmark-1Mkeys-generic-scan-count-500-pipeline-10|memtier_benchmark-1Mkeys-hash-hexists|memtier_benchmark-1Mkeys-hash-hincrby|memtier_benchmark-1Mkeys-hash-hincrbyfloat|memtier_benchmark-1Mkeys-string-decr|memtier_benchmark-1key-list-10K-elements-linsert-lrem-string|memtier_benchmark-1key-zrank-100K-elements-pipeline-1|memtier_benchmark-1key-zset-1M-elements-zremrangebyscore-pipeline-10|memtier_benchmark-nokeys-pubsub-publish-1K-channels-10B-no-subscribers Full Results table:
|
Collaborator
|
@skaslev i've manually confirmed by picking one of the benchmarks that the results are indeed easy to reproduce/see outside of automation, from 13ms to 10ms p50: Results for unstable f6f1674
Results for skaslev/iterator-init ded5aa3
|
Merged
sundb
added a commit
that referenced
this pull request
Aug 4, 2025
This is the General Availability release of Redis Open Source 8.2. ### Major changes compared to 8.0 - Streams - new commands: `XDELEX` and `XACKDEL`; extension to `XADD` and `XTRIM` - Bitmap - `BITOP`: new operators: `DIFF`, `DIFF1`, `ANDOR`, and `ONE` - Query Engine - new SVS-VAMANA vector index type which supports vector compression - More than 15 performance and resource utilization improvements - New metrics: per-slot usage metrics, key size distributions for basic data types, and more ### Binary distributions - Alpine and Debian Docker images - https://hub.docker.com/_/redis - Install using snap - see https://github.com/redis/redis-snap - Install using brew - see https://github.com/redis/homebrew-redis - Install using RPM - see https://github.com/redis/redis-rpm - Install using Debian APT - see https://github.com/redis/redis-debian ### Operating systems we test Redis 8.2 on - Ubuntu 22.04 (Jammy Jellyfish), 24.04 (Noble Numbat) - Rocky Linux 8.10, 9.5 - AlmaLinux 8.10, 9.5 - Debian 12 (Bookworm) - macOS 13 (Ventura), 14 (Sonoma), 15 (Sequoia) ### Security fixes (compared to 8.2-RC1) - (CVE-2025-32023) Fix out-of-bounds write in `HyperLogLog` commands - (CVE-2025-48367) Retry accepting other connections even if the accepted connection reports an error ### New Features (compared to 8.2-RC1) - #14141 Keyspace notifications - new event types: - `OVERWRITTEN` - the value of a key is completely overwritten - `TYPE_CHANGED` - key type change ### Bug fixes (compared to 8.2-RC1) - #14162 Crash when using evport with I/O threads - #14163 `EVAL` crash when error table is empty - #14144 Vector sets - RDB format is not compatible with big endian machines - #14165 Endless client blocking for blocking commands - #14164 Prevent `CLIENT UNBLOCK` from unblocking `CLIENT PAUSE` - #14216 TTL was not removed by the `SET` command - #14224 `HINCRBYFLOAT` removes field expiration on replica ### Performance and resource utilization improvements (compared to 8.2-RC1) - #14200 Store iterators on stack instead of on heap - #14144 Vector set - improve RDB loading / RESTORE speed by storing the worst link info - #Q6430 More compression variants for the SVS-VAMANA vector index - #Q6535 `SHARD_K_RATIO` parameter - favor network latency over accuracy for KNN vector query in a Redis cluster (unstable feature) (MOD-10359) ### Modules API - #14051 `RedisModule_Get*`, `RedisModule_Set*` - allow modules to access Redis configurations - #14114 `RM_UnsubscribeFromKeyspaceEvents` - unregister a module from specific keyspace notifications
skaslev
added a commit
that referenced
this pull request
Nov 21, 2025
…#14473) The PR is follow up on #14200 where we prefer storing iterators on the stack rather than allocating on the heap. Here we continue this for iterators over hashes, lists, sets and kvstores. Quicklist's iterators are still using heap allocation and will be addressed soon. The reason is that `NULL` is perfectly valid quicklist iterator value and handling this would be better reviewed separately from the mostly mechanical changes here.
This was referenced Dec 30, 2025
This was referenced Jan 13, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor use of
dictGetIterator()/dictSafeGetIterator()/listGetIterator()todictInitIterator()/dictInitSafeIterator()/listRewind()respectively which don't allocate memory.