[MOD-10084] port metric iterator to rust by BenGoldberger · Pull Request #6983 · RediSearch/RediSearch

BenGoldberger · 2025-10-06T11:01:56Z

add the metric iterator implementation in rust

Iterator - Metric - Read Dense/C    time:   [20.131 ms 21.760 ms 23.450 ms]
Iterator - Metric - Read Dense/Rust time:   [11.689 ms 11.726 ms 11.771 ms]

Iterator - Metric - Read Sparse/C    time:   [1.0607 ms 1.0628 ms 1.0664 ms]
Iterator - Metric - Read Sparse/Rust time:   [11.645 ms 11.652 ms 11.660 ms]

Iterator - Metric - SkipTo Dense/C    time:   [137.25 µs 138.99 µs 142.01 µs]
Iterator - Metric - SkipTo Dense/Rust time:   [224.61 µs 225.13 µs 225.67 µs]

Iterator - Metric - SkipTo Sparse/C    time:   [5.1269 ms 5.1291 ms 5.1311 ms]
Iterator - Metric - SkipTo Sparse/Rust time:   [17.315 ms 17.324 ms 17.334 ms]

Note

Introduces a Rust Metric iterator (with C interop), updates IdList helpers, exposes metric reset/array length symbols, and adds tests/benchmarks plus minor build/linking tweaks.

Rust Iterators (rqe_iterators):
- Add metric::Metric iterator yielding metric results and attaching RSYieldableMetric values.
- Enhance id_list::IdList with read_and_get_offset/skip_to_and_get_offset to support metric iterator.
- New tests (tests/metric.rs) with C symbol mocks; feature-gated build script links arr for tests.
Benchmarks (rqe_iterators_bencher):
- Add Metric iterator benchmarks and wire into benches/iterators.rs.
- Extend C FFI wrapper with QueryIterator::new_metric and required symbols.
C/FFI:
- Export ResultMetrics_Reset_func(RSIndexResult*) and bind it in Rust (inverted_index re-exports).
- Expose array_len_func in util/arr and include arr.h in Rust FFI bindings; add C++ test for it.
- Provide RSValue/metrics free/reset stubs in tests/benchers to manage metric values.
Build:
- In build.sh, on non-macos add -C link-args=-lgcov for coverage; minor RUSTFLAGS invocation tweak.

^{Written by Cursor Bugbot for commit ce7e57e. This will update automatically on new commits. Configure here.}

…arch/RediSearch into beng-rust-metric-iterator

…l be valid

…beng-rust-metric-iterator

src/util/arr/arr.h

src/redisearch_rs/rqe_iterators/src/metric.rs

…beng-rust-metric-iterator

codecov · 2025-11-03T11:31:22Z

Codecov Report

❌ Patch coverage is 91.75258% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.15%. Comparing base (e777a29) to head (ce7e57e).
⚠️ Report is 10 commits behind head on master.

Files with missing lines	Patch %	Lines
src/redisearch_rs/rqe_iterators/src/metric.rs	94.20%	2 Missing and 2 partials ⚠️
src/index_result.c	0.00%	2 Missing ⚠️
src/redisearch_rs/rqe_iterators/src/id_list.rs	91.66%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6983      +/-   ##
==========================================
- Coverage   85.41%   85.15%   -0.26%     
==========================================
  Files         337      340       +3     
  Lines       51920    52234     +314     
  Branches    11848    12157     +309     
==========================================
+ Hits        44345    44480     +135     
- Misses       7390     7566     +176     
- Partials      185      188       +3

Flag	Coverage Δ
flow	`84.11% <0.00%> (-0.13%)`	⬇️
unit	`52.01% <91.75%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gdesmott · 2025-11-03T12:38:31Z

Iterator - Metric - Read Dense/C time: [20.131 ms 21.760 ms 23.450 ms]
Iterator - Metric - Read Dense/Rust time: [11.689 ms 11.726 ms 11.771 ms]

Iterator - Metric - Read Sparse/C time: [1.0607 ms 1.0628 ms 1.0664 ms]
Iterator - Metric - Read Sparse/Rust time: [11.645 ms 11.652 ms 11.660 ms]

Iterator - Metric - SkipTo Dense/C time: [137.25 µs 138.99 µs 142.01 µs]
Iterator - Metric - SkipTo Dense/Rust time: [224.61 µs 225.13 µs 225.67 µs]

Iterator - Metric - SkipTo Sparse/C time: [5.1269 ms 5.1291 ms 5.1311 ms]
Iterator - Metric - SkipTo Sparse/Rust time: [17.315 ms 17.324 ms 17.334 ms]

The Rust implementation is significantly slower than the C one.

I'm pretty sure it's because of the 2 ffi calls in set_result_metrics (ResultMetrics_Reset_func and array_ensure_append_n_func) which cannot be inlined.

I did some profiling and indeed we see most of the time is spent there.

So I'm afraid there is not much we can do at this point. The best way to solve this would be enable C/Rust LTO (MOD-10751) and/or to port RSYieldableMetric to Rust which requires RSValue to be ported first (MOD-11411).

I'd suggest we merge this for now and open another task reminding us to re-check performance once progress has been made on those other fronts.

@GuyAv46 : what do you think?

BenGoldberger · 2025-11-03T14:13:20Z

@gdesmott do we know why we need all the C mocks in the test?

gdesmott · 2025-11-03T14:17:46Z

@gdesmott do we know why we need all the C mocks in the test?

No, but it seems it's not a new problem with coverage, see https://github.com/RediSearch/RediSearch/blob/master/build.sh#L576

I hope we'll find soon a better way to manage C code from Rust tests and benches.

gdesmott · 2025-11-03T14:46:27Z

I'd suggest we merge this for now and open another task reminding us to re-check performance once progress has been made on those other fronts.

Opened MOD-12235 for this.

GuyAv46

STAMPA

BenGoldberger added 4 commits October 6, 2025 11:34

start working on metric iterator

793c6a3

start working on metric iterator

b2b488d

Merge branch 'beng-rust-metric-iterator' of https://github.com/RediSe…

9d0a67e

…arch/RediSearch into beng-rust-metric-iterator

implement other functions

9e0c111

github-actions bot added the size:M label Oct 6, 2025

BenGoldberger added 3 commits October 8, 2025 09:39

improve metric iterator implementation

93d3229

fix arr binding

0e47850

add benches for metric

5caa471

github-actions bot added the size:L label Oct 9, 2025

BenGoldberger added the action:run-rust-micro-benchmark label Oct 9, 2025

BenGoldberger added 9 commits October 13, 2025 01:58

add array func and change cap and elem_size so that rust pointers wil…

58e60fa

…l be valid

clean up

bc191b2

bind redis alloc, and mock rs_value

54069f3

Merge remote-tracking branch 'origin/beng-changes-to-array-lib' into …

9a7e37d

…beng-rust-metric-iterator

test metric vals in skip to

d655203

fmt

0bac778

improve test

09b1305

change signature

f982cae

add metric iter benches

955d2e0

gdesmott reviewed Oct 13, 2025

View reviewed changes

src/util/arr/arr.h Outdated Show resolved Hide resolved

src/redisearch_rs/rqe_iterators/src/metric.rs Outdated Show resolved Hide resolved

add trub implementation for rs_value_decref

83c8b73

BenGoldberger changed the title ~~Beng-rust-metric-iterator~~ [MOD-10084] Beng-rust-metric-iterator Oct 13, 2025

BenGoldberger added 8 commits October 13, 2025 12:47

fix clear and remain_cap

d16dc2c

improve rs_value_mock

30b5a49

Merge remote-tracking branch 'origin/beng-changes-to-array-lib' into …

56c5664

…beng-rust-metric-iterator

fix bench, add rsvalue_newnumber mock

72d99dd

more tests

f933c7d

start working on metric iterator

d42e06f

implement other functions

8e7bd6f

improve metric iterator implementation

79a37f6

fix coverage link on Rust tests requiring C code

ecc8157

gdesmott force-pushed the beng-rust-metric-iterator branch from 0b6783c to ecc8157 Compare November 3, 2025 11:44

gdesmott previously approved these changes Nov 3, 2025

View reviewed changes

gdesmott enabled auto-merge November 3, 2025 14:52

GuyAv46 previously approved these changes Nov 3, 2025

View reviewed changes

gdesmott added this pull request to the merge queue Nov 3, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 3, 2025

try to fix macos

ce7e57e

BenGoldberger dismissed stale reviews from GuyAv46 and gdesmott via ce7e57e November 4, 2025 08:57

BenGoldberger added the MacOS label Nov 4, 2025

BenGoldberger requested review from GuyAv46 and gdesmott November 4, 2025 09:47

BenGoldberger enabled auto-merge November 4, 2025 10:09

alonre24 approved these changes Nov 4, 2025

View reviewed changes

BenGoldberger added this pull request to the merge queue Nov 4, 2025

Merged via the queue into master with commit b7ac496 Nov 4, 2025
24 checks passed

BenGoldberger deleted the beng-rust-metric-iterator branch November 4, 2025 12:17

BenGoldberger changed the title ~~[MOD-10084] Beng-rust-metric-iterator~~ [MOD-10084] port metric iterator to rust Nov 4, 2025

BenGoldberger added size:L and removed MacOS size:M size:L size:XL labels Nov 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MOD-10084] port metric iterator to rust#6983

[MOD-10084] port metric iterator to rust#6983
BenGoldberger merged 109 commits intomasterfrom
beng-rust-metric-iterator

BenGoldberger commented Oct 6, 2025 •

edited by cursor bot

Loading

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Nov 3, 2025 •

edited

Loading

Uh oh!

gdesmott commented Nov 3, 2025

Uh oh!

BenGoldberger commented Nov 3, 2025

Uh oh!

gdesmott commented Nov 3, 2025

Uh oh!

gdesmott commented Nov 3, 2025

Uh oh!

GuyAv46 left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

BenGoldberger commented Oct 6, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gdesmott commented Nov 3, 2025

Uh oh!

BenGoldberger commented Nov 3, 2025

Uh oh!

gdesmott commented Nov 3, 2025

Uh oh!

gdesmott commented Nov 3, 2025

Uh oh!

GuyAv46 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

BenGoldberger commented Oct 6, 2025 •

edited by cursor bot

Loading

codecov bot commented Nov 3, 2025 •

edited

Loading