feat(rhai): add intern_strings config option to allow elimination of RwLock contention under concurrent load (copy #9070)

.changesets/feat_rhai_max_strings_interned.md

@@ -0,0 +1,18 @@
+### Add `intern_strings` configuration option for the Rhai plugin
+
+The Rhai plugin now exposes an `intern_strings` option that controls Rhai's internal string interning. Under high concurrency, threads encountering new strings must acquire a write lock, which can serialize Rhai execution across concurrent requests.
+
+Setting `intern_strings: false` disables interning, eliminating the lock:
+
+```yaml
+rhai:
+  scripts: ./rhai
+  main: main.rhai
+  intern_strings: false
+```
+
+String interning can alleviate memory allocation and make string equality checks a little faster. For deployments serving many concurrent requests, the cost likely outweighs the benefit, so we recommend experimenting with `intern_strings: false` and observing if it improves performance.
+
+The default (`true`) preserves the existing behaviour.
+
+By [@theJC](https://github.com/theJC) in https://github.com/apollographql/router/pull/9070

.gitleaks.toml

@@ -5,6 +5,7 @@
             '''Cargo.lock$'''
             ,'''^\.changesets\/.+\.md$'''
             ,'''^CHANGELOG\.md$'''
+            ,'''^apollo-router-benchmarks/benches/rhai_string_interning\.rs$'''
         ]
 
 [[ rules ]]

Cargo.lock

@@ -469,6 +469,7 @@ dependencies = [
  "criterion",
  "memory-stats",
  "once_cell",
+ "rhai",
  "serde_json",
  "tokio",
  "tower",

apollo-router-benchmarks/Cargo.toml

@@ -11,6 +11,7 @@ apollo-router = { path = "../apollo-router" }
 criterion = { version = "0.8", features = ["async_tokio", "async_futures"] }
 memory-stats = "1.1.0"
 once_cell.workspace = true
+rhai = { version = "1.23.6", features = ["sync", "serde", "internals"] }
 serde_json.workspace = true
 tokio.workspace = true
 tower.workspace = true
@@ -27,3 +28,7 @@ harness = false
 [[bench]]
 name = "memory_use"
 harness = false
+
+[[bench]]
+name = "rhai_string_interning"
+harness = false

apollo-router-benchmarks/benches/fixtures/rhai/string_heavy.rhai

@@ -0,0 +1,26 @@
+// String-heavy Rhai script for benchmarking string interning performance.
+//
+// This script is intentionally designed to exercise the Rhai string interner
+// frequently: each iteration of the loop performs string concatenation and map
+// key lookups, both of which acquire the RwLock on the StringsInterner when
+// the `sync` feature is enabled.
+//
+// The goal is to produce a measurable difference between default interning
+// (256 strings, RwLock acquired per op) and disabled interning (0, no lock).
+
+fn supergraph_service(service) {
+    let request_callback = Fn("process_request");
+    service.map_request(request_callback);
+}
+
+fn process_request(request) {
+    let prefix = "x-rhai-bench-";
+    let i = 0;
+    while i < 20 {
+        let key = prefix + to_string(i);
+        let val = "bench-value-" + to_string(i) + "-done";
+        request.headers[key] = val;
+        i += 1;
+    }
+    request.context["rhai_bench_complete"] = "true";
+}

apollo-router-benchmarks/benches/rhai_string_interning.rs

@@ -0,0 +1,239 @@
+//! Benchmark: Rhai string interner — direct engine, no router stack.
+//!
+//! Tests whether disabling Rhai's `RwLock<StringsInterner>` (via
+//! `engine.set_max_strings_interned(0)`) improves throughput under concurrent
+//! load for scripts representative of production request processing.
+//!
+//! The script is modelled on the real subgraph request callback work:
+//!   - Lookups in a context map using long string keys
+//!     (`"apollo::authentication::jwt_status"`, `"customer::client_name"`, ...)
+//!   - `in` operator containment checks (string equality under the hood)
+//!   - Template-string interpolation (`\`...\``)
+//!   - String comparisons (`starts_with`, `==`)
+//!   - Cookie / CSV building loops (string concatenation)
+//!
+//! Two configurations:
+//!   - `default_256_interned` — `Engine::new()` default, 256-entry interner,
+//!     every string op acquires `RwLock<StringsInterner>`
+//!   - `disabled_0_interned` — `set_max_strings_interned(0)`, interner field is
+//!     `None`, no lock is ever taken
+//!
+//! Two variants:
+//!   - `sequential` — single thread, measures raw Rhai execution cost
+//!   - `concurrent_N` — N OS threads sharing one `Arc<Engine>`, surfaces any
+//!     RwLock write contention on the interner
+//!
+//! Run with:
+//! ```
+//! cargo bench -p apollo-router-benchmarks --bench rhai_string_interning
+//! ```
+
+use std::sync::Arc;
+use std::time::Duration;
+use std::time::Instant;
+
+use criterion::criterion_group;
+use criterion::criterion_main;
+use criterion::BenchmarkId;
+use criterion::Criterion;
+use criterion::Throughput;
+use rhai::Engine;
+use rhai::Scope;
+
+// How many OS threads share the engine in the concurrent variant.
+const CONCURRENCY: usize = 8;
+
+// A Rhai script representative of typical subgraph request callback work.
+//
+// Patterns exercised:
+//   - Long context-key string literals (`"apollo::authentication::jwt_status"`, ...)
+//   - Map construction and `[]`-key lookups
+//   - `in` containment operator (string equality)
+//   - `if` / `else if` chains comparing strings
+//   - Template-string interpolation
+//   - Cookie-building loop with string concatenation
+//   - `starts_with` / `len` string methods
+const SCRIPT: &str = r#"
+    // ---------- context simulation ----------------------------------------
+    // Mirrors a ~40-key context map accessed across typical request callbacks.
+    let ctx = #{
+        "apollo::authentication::jwt_status": (),
+        "apollo::authentication::jwt_claims": (),
+        "apollo::supergraph::operation_kind": "query",
+        "apollo::supergraph::operation_name": "TopProducts",
+        "customer::caller_tag": "test-client-service",
+        "customer::caller_cred_kind": "oauth",
+        "customer::caller_grants": ["role-a", "role-b"],
+        "customer::has_override": false,
+        "customer::format": "en_US",
+        "customer::rg": "us",
+        "customer::vault_gate_status": "present",
+        "customer::vault_gate_bearer": "Bearer eyJ...",
+        "customer::perms::level_resolved": (),
+        "customer::entity_ref_status": "present",
+        "customer::entity_ref": "acct-0123456789",
+        "customer::entity_ref_resolver": "identity-service",
+        "customer::tokens::validation_passed": #{ "TOKN": "abc123value", "SKEY": "xyz789value" },
+        "customer::tokens::org_scope": (),
+        "customer::tokens::emitted": (),
+        "customer::net_origin": "203.0.113.42",
+        "customer::req_epoch_ms": 1_700_000_000_000
+    };
+
+    // ---------- authentication check equivalent ---------------------------
+    let has_auth = "customer::caller_cred_kind" in ctx;
+
+    // ---------- retry header equivalent -----------------------------------
+    let op_kind = ctx["apollo::supergraph::operation_kind"];
+    let idempotency = if op_kind == "query" { "true" } else { "" };
+
+    // ---------- ext auth forwarding equivalent ----------------------------
+    let ext_auth_header = "";
+    if "customer::vault_gate_status" in ctx {
+        let status = ctx["customer::vault_gate_status"];
+        if status == "failure" {
+            ext_auth_header = "customer-vault-gate-failure: true";
+        } else if status == "invalid" {
+            ext_auth_header = "customer-vault-gate-invalid: true";
+        } else if status == "present" && "customer::vault_gate_bearer" in ctx {
+            ext_auth_header = `customer-vault-gate: ${ctx["customer::vault_gate_bearer"]}`;
+        }
+    }
+
+    // ---------- entity ref forwarding equivalent --------------------------
+    let acct_header = "";
+    if "customer::entity_ref_status" in ctx {
+        let ref_status = ctx["customer::entity_ref_status"];
+        if ref_status == "failure" {
+            acct_header = "customer-svc-entity-ref-failure: true";
+        } else if ref_status == "invalid" {
+            acct_header = "customer-svc-entity-ref-invalid: true";
+        } else if ref_status == "present" {
+            let ref_id = ctx["customer::entity_ref"];
+            let ref_ns = ctx["customer::entity_ref_resolver"];
+            acct_header = `customer-svc-entity-ref: ${ref_id} / ${ref_ns}`;
+        }
+    }
+
+    // ---------- region/format query-param forwarding equivalent ----------
+    let query_string = "";
+    if "customer::rg" in ctx {
+        query_string += `?rg=${ctx["customer::rg"]}`;
+    }
+    if "customer::format" in ctx {
+        if query_string == "" {
+            query_string += `?fmt=${ctx["customer::format"]}`;
+        } else {
+            query_string += `&fmt=${ctx["customer::format"]}`;
+        }
+    }
+    let path = `/data-retrieval-service/graphql${query_string}`;
+
+    // ---------- token forwarding equivalent (build loop) -----------------
+    let valid_tokens = ctx["customer::tokens::validation_passed"];
+    let cookie_string = "";
+    if valid_tokens != () {
+        for key in valid_tokens.keys() {
+            let value = valid_tokens[key];
+            if cookie_string != "" { cookie_string += "; "; }
+            cookie_string += `${key}=${value}`;
+        }
+    }
+
+    // ---------- token subject check equivalent ---------------------------
+    // (starts_with + len — mirrors sub-claim validation pattern)
+    let sub = "app:abcdefgh1234567890abcdefgh1234567890abcdefgh1234567890abcdefgh";
+    let is_app_sub = sub.starts_with("app:") && sub.len() == 68;
+
+    // ---------- result (prevents dead-code elimination) ------------------
+    #{
+        has_auth: has_auth,
+        idempotency: idempotency,
+        ext_auth_header: ext_auth_header,
+        acct_header: acct_header,
+        path: path,
+        cookie_string: cookie_string,
+        is_app_sub: is_app_sub
+    }
+"#;
+
+fn make_engine(max_strings_interned: Option<usize>) -> Arc<Engine> {
+    let mut engine = Engine::new();
+    if let Some(n) = max_strings_interned {
+        engine.set_max_strings_interned(n);
+    }
+    Arc::new(engine)
+}
+
+fn rhai_string_interning_benchmark(c: &mut Criterion) {
+    let configs: &[(&str, Option<usize>)] = &[
+        ("default_256_interned", None),
+        ("disabled_0_interned", Some(0)),
+    ];
+
+    for &(label, max_strings_interned) in configs {
+        let engine = make_engine(max_strings_interned);
+        let ast = Arc::new(engine.compile(SCRIPT).expect("script compiles"));
+
+        let mut group = c.benchmark_group("rhai_string_interning");
+        group
+            .measurement_time(Duration::from_secs(20))
+            .sample_size(200)
+            .throughput(Throughput::Elements(1));
+
+        // --- Sequential: one scope per iteration, single thread ----------
+        {
+            let engine = engine.clone();
+            let ast = ast.clone();
+            group.bench_with_input(BenchmarkId::new("sequential", label), label, |b, _| {
+                b.iter(|| {
+                    let mut scope = Scope::new();
+                    engine
+                        .eval_ast_with_scope::<rhai::Dynamic>(&mut scope, &ast)
+                        .expect("eval ok")
+                });
+            });
+        }
+
+        // --- Concurrent: CONCURRENCY threads sharing one engine ----------
+        // `iter_custom` lets us drive N threads per criterion iteration and
+        // report wall-clock time for the whole batch.  Each thread runs
+        // `per_thread` iterations so the reported time covers all of them.
+        {
+            let engine = engine.clone();
+            let ast = ast.clone();
+            group.bench_with_input(
+                BenchmarkId::new(format!("concurrent_{CONCURRENCY}"), label),
+                label,
+                |b, _| {
+                    b.iter_custom(|iters| {
+                        let per_thread = (iters as usize).max(1).div_ceil(CONCURRENCY);
+                        let engine = engine.clone();
+                        let ast = ast.clone();
+                        let start = Instant::now();
+                        std::thread::scope(|s| {
+                            for _ in 0..CONCURRENCY {
+                                let engine = engine.clone();
+                                let ast = ast.clone();
+                                s.spawn(move || {
+                                    for _ in 0..per_thread {
+                                        let mut scope = Scope::new();
+                                        let _ = engine
+                                            .eval_ast_with_scope::<rhai::Dynamic>(&mut scope, &ast)
+                                            .expect("eval ok");
+                                    }
+                                });
+                            }
+                        });
+                        start.elapsed()
+                    });
+                },
+            );
+        }
+
+        group.finish();
+    }
+}
+
+criterion_group!(benches, rhai_string_interning_benchmark);
+criterion_main!(benches);

apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap

@@ -1,5 +1,6 @@
 ---
 source: apollo-router/src/configuration/tests.rs
+assertion_line: 28
 expression: "&schema"
 ---
 {
@@ -8331,6 +8332,11 @@ expression: "&schema"
       "additionalProperties": false,
       "description": "Configuration for the Rhai Plugin",
       "properties": {
+        "intern_strings": {
+          "default": true,
+          "description": "Whether to enable Rhai's internal string interning.\n\nWhen the `sync` feature is active (required for multi-threaded use),\nevery string operation acquires a `RwLock` on the interner. Under high\nconcurrency this lock can become a bottleneck.\n\nSet to `false` to disable string interning entirely, which eliminates\nlock acquisition on every string operation and can improve throughput\nfor workloads with many concurrent Rhai executions.\n\nDefaults to `true`, which preserves Rhai's built-in default of 256\ninterned strings.",
+          "type": "boolean"
+        },
         "main": {
           "description": "The main entry point for Rhai script evaluation",
           "type": [

apollo-router/src/plugins/rhai/engine/mod.rs

@@ -1390,8 +1390,16 @@ impl Rhai {
             .register_iterator::<HeaderMap>();
     }
 
-    pub(super) fn new_rhai_engine(path: Option<PathBuf>, sdl: String, main: PathBuf) -> Engine {
+    pub(super) fn new_rhai_engine(
+        path: Option<PathBuf>,
+        sdl: String,
+        main: PathBuf,
+        intern_strings: bool,
+    ) -> Engine {
         let mut engine = Engine::new();
+        if !intern_strings {
+            engine.set_max_strings_interned(0);
+        }
         // If we pass in a path, use it to configure our engine
         // with a FileModuleResolver which allows import to work
         // in scripts.

apollo-router/src/plugins/rhai/mod.rs

@@ -50,6 +50,10 @@ struct Rhai {
     scope: Arc<Mutex<Scope<'static>>>,
 }
 
+fn default_intern_strings() -> bool {
+    true
+}
+
 /// Configuration for the Rhai Plugin
 #[derive(Deserialize, JsonSchema)]
 #[serde(deny_unknown_fields)]
@@ -59,6 +63,17 @@ pub(crate) struct Conf {
     scripts: Option<PathBuf>,
     /// The main entry point for Rhai script evaluation
     main: Option<String>,
+    /// Whether to enable Rhai's internal string interning.
+    ///
+    /// String interning can reduce memory allocations and string comparison
+    /// cost. But it also introduces synchronization overhead.
+    ///
+    /// Setting this to `false` can improve throughput and is recommended
+    /// for workloads with many concurrent Rhai executions.
+    ///
+    /// Defaults to `true`.
+    #[serde(default = "default_intern_strings")]
+    intern_strings: bool,
 }
 
 #[async_trait::async_trait]
@@ -83,6 +98,7 @@ impl Plugin for Rhai {
             Some(scripts_path),
             sdl.to_string(),
             main.clone(),
+            init.config.intern_strings,
         ));
         let ast = engine
             .compile_file(main.clone())