MOD-17912 Improve get_index performance (backport to 8.8) - #1633
Conversation
* MOD-17912 Improve `get_index` performance * fmt (cherry picked from commit 21568f4)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.8 #1633 +/- ##
=======================================
Coverage 75.23% 75.24%
=======================================
Files 15 15
Lines 4252 4253 +1
=======================================
+ Hits 3199 3200 +1
Misses 1053 1053 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 27d9bdc. Configure here.
| @@ -1,2 +1,3 @@ | |||
| [toolchain] | |||
| channel = "1.92" | |||
| components = ["rust-analyzer"] No newline at end of file | |||
There was a problem hiding this comment.
Unrelated toolchain change in backport
Low Severity
This backport adds components = ["rust-analyzer"] to rust-toolchain.toml, which is unrelated to the get_index performance fix. Rustup will pull that LSP component on toolchain sync for every checkout and CI job that honors the file, adding install overhead with no effect on the RedisJSON module.
Reviewed by Cursor Bugbot for commit 27d9bdc. Configure here.
1 participant
Backport of 21568f4 (#1631) to
8.8.IValue::get_indexwasarr.iter().nth(index).ArrayIterimplements onlynext(), sonth()falls back to theIteratordefault and walks the array one element at a time; on a packed typed array each skipped element is also materialized into an ownedIValue(a heap allocation). Reading a whole vector was therefore quadratic. This is the LLAPIgetAtpath RediSearch uses to ingest JSON vector fields — see RED-213492, where dim-1280 embeddings cost ~16 ms per vector and ~4,300 s of a ~4,500 s RDB load.Cherry-picked cleanly, no adaptation needed.
Test plan
cargo test -p json_path— passes (8.8)cargo fmt --checkclean🤖 Generated with Claude Code
Note
Low Risk
Localized indexing change with tests; behavior should match except for much less allocation. Not auth or data-integrity logic.
Overview
Makes
IValue::get_indexO(1) by indexing the packed backing slice instead of walkingarr.iter().nth(index). That iterator only implementsnext(), so a full scan of a typed array was quadratic and allocated an ownedIValuefor every skipped element.This is the LLAPI
getAtpath used to ingest JSON vector fields (e.g. RediSearch embeddings). Heterogeneous arrays stay borrowed; typed numeric arrays still return owned values, but only for the requested index.Adds coverage for heterogeneous, F32, and I64 arrays, plus a non-array case. Also adds
rust-analyzertorust-toolchain.toml.Reviewed by Cursor Bugbot for commit 27d9bdc. Bugbot is set up for automated code reviews on this repo. Configure here.