Rust: take Session::read_bulk tag list by borrowed slice

Changes the public signature from `Vec<String>` (owned) to
`&[impl AsRef<str>]` so callers can re-issue the same call repeatedly
without cloning at the call site. The bench''s steady-state loop now
passes `&tags` instead of `tags.clone()`; the CLI subcommand passes the
parsed `&items`; the integration test passes `&["Area001.Pump001.Speed"]`
straight from a string literal slice.

Honest perf note: this is an ergonomics change, not a measurable speedup.
The method still has to materialise an owned `Vec<String>` internally
because prost''s generated `ReadBulkCommand` field requires it, so the
total heap traffic per call is unchanged. Across two 30-second, 5-way
concurrent bench runs at bulkSize=6:

  pre-fix  (.clone() at caller): 145.35 calls/sec, p99  62.31 ms
  post-fix run 1 (&tags):        165.98 calls/sec, p99  40.65 ms
  post-fix run 2 (&tags):        146.19 calls/sec, p99  60.04 ms

Run-to-run variance (145-166) dominates any signal from the fix. Solo
Rust release stayed at 261-267 calls/sec across both API shapes,
confirming the bench is gateway-bound under 5-way contention rather
than client-allocation-bound. The change is kept because the borrowed
slice is the idiomatic Rust API shape for "list of items the callee
does not need to take ownership of", and it cleans up the explicit
clone from the bench's inner loop.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-05-20 05:49:33 -04:00
parent 7db4bffa30
commit 61644e63fb
3 changed files with 18 additions and 6 deletions
+3 -3
View File
@@ -568,7 +568,7 @@ async fn run(cli: Cli) -> Result<(), Error> {
json,
} => {
let session = session_for(connection, session_id).await?;
let results = session.read_bulk(server_handle, items, timeout_ms).await?;
let results = session.read_bulk(server_handle, &items, timeout_ms).await?;
print_read_bulk_results("read-bulk", &results, json);
}
Command::WriteBulk {
@@ -1038,7 +1038,7 @@ async fn run_bench_read_bulk(
+ std::time::Duration::from_secs(warmup_seconds);
while std::time::Instant::now() < warmup_deadline {
let _ = session
.read_bulk(server_handle, tags.clone(), timeout_ms)
.read_bulk(server_handle, &tags, timeout_ms)
.await;
}
@@ -1052,7 +1052,7 @@ async fn run_bench_read_bulk(
while std::time::Instant::now() < steady_deadline {
let call_start = std::time::Instant::now();
let outcome = session.read_bulk(server_handle, tags.clone(), timeout_ms).await;
let outcome = session.read_bulk(server_handle, &tags, timeout_ms).await;
let elapsed_ms = call_start.elapsed().as_secs_f64() * 1000.0;
latencies_ms.push(elapsed_ms);
match outcome {
+14 -2
View File
@@ -477,16 +477,28 @@ impl Session {
/// Per-tag failures appear as `BulkReadResult` entries with
/// `was_successful = false`; the call never errors on per-tag failure.
///
/// `tag_addresses` is taken by borrowed slice so callers can re-issue the
/// same call repeatedly (typical for the bench loop and for any caller
/// polling a fixed snapshot set) without owning or cloning the list at the
/// call site. The method still has to materialise an owned `Vec<String>`
/// internally because prost's `ReadBulkCommand` field requires it, so this
/// is a clarity-of-ownership change rather than an allocation-reducing
/// one: total heap traffic per call is unchanged.
///
/// # Errors
///
/// Same conditions as [`Session::add_item_bulk`].
pub async fn read_bulk(
pub async fn read_bulk<S: AsRef<str>>(
&self,
server_handle: i32,
tag_addresses: Vec<String>,
tag_addresses: &[S],
timeout_ms: u32,
) -> Result<Vec<BulkReadResult>, Error> {
ensure_bulk_size("tag_addresses", tag_addresses.len())?;
let tag_addresses: Vec<String> = tag_addresses
.iter()
.map(|s| s.as_ref().to_owned())
.collect();
let reply = self
.invoke(
MxCommandKind::ReadBulk,
+1 -1
View File
@@ -154,7 +154,7 @@ async fn read_bulk_forwards_timeout_and_unpacks_cached_flag() {
let session = client.session("session-fixture");
let results = session
.read_bulk(12, vec!["Area001.Pump001.Speed".to_owned()], 750)
.read_bulk(12, &["Area001.Pump001.Speed"], 750)
.await
.unwrap();