Merged
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7843 +/- ##
==========================================
- Coverage 84.22% 84.17% -0.05%
==========================================
Files 360 361 +1
Lines 56225 56283 +58
Branches 15717 15720 +3
==========================================
+ Hits 47356 47379 +23
- Misses 8653 8688 +35
Partials 216 216
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Automated performance analysis summaryThis comment was automatically generated given there is performance data available. In summary:
You can check a comparison in detail via the grafana link Performance Improvements - Comparison between master and guyav-refactor_keys_dict.Time Period from 30 days ago. (environment used: oss-standalone)
Performance Regressions and Issues - Comparison between master and guyav-refactor_keys_dict.Time Period from 30 days ago. (environment used: oss-standalone)
Tests with No Significant Changes (30 tests)Tests with No Significant Changes
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the internal key dictionary (keysDict) to use plain CharBuf structures instead of RedisModuleString for keys, and moves vector index storage from the shared dictionary to individual FieldSpec structures. This optimization eliminates temporary string allocations during query parsing.
Key changes:
- Introduced
CharBuftype and custom dictionary functions for hash/compare/duplicate/destroy operations - Modified
IndexSpec_GetFormattedKey*functions to returnCharBuf*instead ofRedisModuleString* - Moved vector index storage from
keysDicttoFieldSpec.vectorOpts.vecIndex, managed per-field
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/spec.h | Added CharBuf struct definition and updated IndexSpecFmtStrings to use CharBuf* |
| src/spec.c | Implemented CharBuf dict operations; updated IndexSpec_GetFormattedKey* to return CharBuf*; updated cleanup logic |
| src/field_spec.h | Added vecIndex pointer to vectorOpts for per-field vector index storage |
| src/field_spec.c | Added cleanup for vecIndex in FieldSpec_Cleanup |
| src/vector_index.h | Changed openVectorIndex to take FieldSpec* instead of key name |
| src/vector_index.c | Simplified openVectorIndex to access field-level vecIndex directly; removed dict-based lookup |
| src/tag_index.h/c | Updated TagIndex_Open to accept CharBuf*; updated callers |
| src/numeric_index.h/c | Updated openNumericKeysDict to accept CharBuf*; updated callers |
| src/geometry_index.c | Updated openGeometryKeysDict to accept CharBuf* |
| src/redis_index.h/c | Updated Redis_OpenReader to take RSToken* and token id; renamed legacy functions; added Redis_LegacyDeleteKey |
| src/obfuscation/hidden.h/c | Renamed HiddenString_DropFromKeySpace to HiddenString_LegacyDropFromKeySpace |
| src/query.c | Updated query evaluation to pass tokens directly to Redis_OpenReader |
| src/module.c | Updated to use new CharBuf* based APIs |
| src/document.c | Updated indexers to use new API signatures |
| src/indexer.c | Updated vector index access for document deletion |
| src/fork_gc.c | Updated GC to use CharBuf for dictionary operations |
| src/debug_commands.c | Updated debug commands to work with field specs and CharBuf* |
| src/info/field_spec_info.c | Updated to use new openVectorIndex signature |
| tests/pytests/test_index.py | Removed vector index count increments (vectors no longer in keysDict) |
| tests/cpptests/*.cpp | Updated tests to use CharBuf* and new API signatures; removed string cleanup calls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
eyalrund
reviewed
Dec 21, 2025
eyalrund
reviewed
Dec 21, 2025
This comment was marked as resolved.
This comment was marked as resolved.
GuyAv46
force-pushed
the
guyav-refactor_keys_dict
branch
from
1d13500 to
35ec166
Compare
GuyAv46
force-pushed
the
guyav-refactor_keys_dict
branch
from
35ec166 to
ccdbe22
Compare
GuyAv46
changed the base branch from
master
to
guyav-deprecate_fields_formatting
2 tasks
GuyAv46
force-pushed
the
guyav-refactor_keys_dict
branch
from
ccdbe22 to
ecd02bc
Compare
ofiryanai
approved these changes
Dec 28, 2025
Contributor
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 8.2
git worktree add -d .worktree/backport-7843-to-8.2 origin/8.2
cd .worktree/backport-7843-to-8.2
git switch --create backport-7843-to-8.2
git cherry-pick -x 692e14fa369df2c41bf1d577cbac92b0c6fee217 |
Contributor
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 8.4
git worktree add -d .worktree/backport-7843-to-8.4 origin/8.4
cd .worktree/backport-7843-to-8.4
git switch --create backport-7843-to-8.4
git cherry-pick -x 692e14fa369df2c41bf1d577cbac92b0c6fee217 |
GuyAv46
added a commit
that referenced
this pull request
Dec 28, 2025
* mark legacy API as such * refactor spec's keysDict to hold char buffer * cleanups * make keysDict a dict for inverted indexes only (simplify value struct) * use bool * some comment fixes * address review comment * dead code cleanup (cherry picked from commit 692e14f)
GuyAv46
added a commit
that referenced
this pull request
Dec 28, 2025
* mark legacy API as such * refactor spec's keysDict to hold char buffer * cleanups * make keysDict a dict for inverted indexes only (simplify value struct) * use bool * some comment fixes * address review comment * dead code cleanup (cherry picked from commit 692e14f)
This was referenced Dec 28, 2025
GuyAv46
added a commit
that referenced
this pull request
Dec 29, 2025
* mark legacy API as such * refactor spec's keysDict to hold char buffer * cleanups * make keysDict a dict for inverted indexes only (simplify value struct) * use bool * some comment fixes * address review comment * dead code cleanup (cherry picked from commit 692e14f)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 29, 2025
* Refactor keys dict - [MOD-13151] (#7843) * mark legacy API as such * refactor spec's keysDict to hold char buffer * cleanups * make keysDict a dict for inverted indexes only (simplify value struct) * use bool * some comment fixes * address review comment * dead code cleanup (cherry picked from commit 692e14f) * CP fix * CP fix for 8.2 * add back missing definition
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 29, 2025
Refactor keys dict - [MOD-13151] (#7843) * mark legacy API as such * refactor spec's keysDict to hold char buffer * cleanups * make keysDict a dict for inverted indexes only (simplify value struct) * use bool * some comment fixes * address review comment * dead code cleanup (cherry picked from commit 692e14f)
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Describe the changes in the pull request
Since our terms are no longer stored in Redis key-space, we don't need to format them before adding them to the spec's keys dictionary.
Avoiding the format will speed up query parsing, especially for complex queries and infix (suffix, prefix) queries, where we traverse many tokens and therefore perform numerous temporary allocations and string copies.
To stop formatting inverted index keys, we need:
keysDictkey type to avoid creating temporaryRedisModuleStrings for lookup.We also now create (and allocate)
NewQueryTermonly if it's needed, avoiding the paths where we allocate and later free the term without using itMark if applicable
Note
Consolidates key handling for in-memory inverted indexes and reduces per-term allocations across query paths.
keysDictnow storesInvertedIndexdirectly keyed byCharBufwith a customdictType(invIdxDictType); removeKeysDictValuewrapper and addCharBufhash/dup/compare/destructors andInvIndFreeCbfor values.IndexSpec_MakeKeylessuses the new dict type;missingFieldDictuses the shared value dtor.Redis_OpenInvertedIndex/openIndexKeysDictaccept rawCharBuf(noRedisModuleString); introduceLegacy_fmtRedis*helpers andRedis_LegacyDropScanHandlerfor old key-space cleanup paths only.Redis_OpenReadernow takesRSTokenandtok_id;RSQueryTermis created only when building the iterator. Update token/prefix/wildcard expansion and disk-iterator sites to pass raw token data and incrementtokenIdappropriately.keysDictusingCharBuf; spec drop logic and iterators switched to legacy formatters only where needed.Written by Cursor Bugbot for commit 4bb4b70. This will update automatically on new commits. Configure here.