MOD-17912 Improve get_index performance (backport to 8.6) - #1634
Conversation
* MOD-17912 Improve `get_index` performance * fmt (cherry picked from commit 21568f4) Test adapted to the 8.6 SelectValue API: get_long/get_double/as_str are non-Option there, and JSONArrayType/get_array_type (V7 getArray, 8.8+) do not exist, so the packed-array assertion uses ijson's ArraySliceRef::is_typed.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.6 #1634 +/- ##
=======================================
Coverage 77.59% 77.59%
=======================================
Files 15 15
Lines 3878 3879 +1
=======================================
+ Hits 3009 3010 +1
Misses 869 869 ☔ 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 2bd8c56. 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 component added
Low Severity
components = ["rust-analyzer"] appears in this backport with no mention in the PR description or test plan. It is unrelated to the get_index fix and forces rustup to install an IDE LSP component whenever the toolchain syncs, which can slow or fail CI/dev installs without changing RedisJSON behavior.
Reviewed by Cursor Bugbot for commit 2bd8c56. Configure here.
1 participant
Backport of 21568f4 (#1631) to
8.6.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.Adaptations for
8.6:SelectValueAPI:get_long/get_double/as_strare non-Option here, andJSONArrayType/get_array_type(V7getArray, 8.8+) do not exist — the packed-array assertion uses ijson'sArraySliceRef::is_typedinstead.The fix itself is byte-identical to master; the ijson pinned here (
5676f592) has the sameArraySliceRefvariants.Test plan
cargo test -p json_path— passes (8.6)cargo fmt --checkclean🤖 Generated with Claude Code
Note
Low Risk
Performance-only indexing change with tests; no auth, persistence, or API contract changes. Typed elements still allocate one owned IValue per lookup, matching prior iterator behavior.
Overview
Fixes quadratic
IValue::get_indexon packed typed arrays by indexing the backingArraySliceRefinstead ofiter().nth(), which walked (and allocated) every skipped element.This is the LLAPI
getAtpath used to ingest JSON vector fields. Heterogeneous arrays still borrow; typed numeric slices wrap a single ownedIValue. Adds coverage for mixed, float, and long arrays, plus a non-array case. Also listsrust-analyzerinrust-toolchain.toml.Reviewed by Cursor Bugbot for commit 2bd8c56. Bugbot is set up for automated code reviews on this repo. Configure here.