Description
core/connectors/sdk/src/convert.rs::owned_value_to_serde_json takes
&simd_json::OwnedValue and recursively allocates a fresh String
for every string leaf (s.to_string()) and every object key
(k.to_string()) it visits:
pub fn owned_value_to_serde_json(value: &simd_json::OwnedValue) -> serde_json::Value {
match value {
simd_json::OwnedValue::Static(s) => match s { /* ... */ },
simd_json::OwnedValue::String(s) => serde_json::Value::String(s.to_string()),
simd_json::OwnedValue::Array(arr) => {
serde_json::Value::Array(arr.iter().map(owned_value_to_serde_json).collect())
}
simd_json::OwnedValue::Object(obj) => {
let map: serde_json::Map<String, serde_json::Value> = obj
.iter()
.map(|(k, v)| (k.to_string(), owned_value_to_serde_json(v)))
.collect();
serde_json::Value::Object(map)
}
}
}
At most call sites the caller already owns the OwnedValue outright
and drops it immediately after converting. Converting between
OwnedValue and serde_json::Value means rebuilding the tree either
way, since they're unrelated types, but the leaf strings and object
keys don't need to ride along with that rebuild: they're already-owned
heap allocations one line away from being dropped, so .to_string()
copies their bytes into a new allocation just to throw the original
away right after.
Affected area / component
Connectors
Proposed solution
Add a consuming variant, additive (no breaking signature change):
pub fn owned_value_into_serde_json(value: simd_json::OwnedValue) -> serde_json::Value {
match value {
simd_json::OwnedValue::Static(s) => match s { /* unchanged, Copy types */ },
simd_json::OwnedValue::String(s) => serde_json::Value::String(s.into()),
simd_json::OwnedValue::Array(arr) => {
serde_json::Value::Array(arr.into_iter().map(owned_value_into_serde_json).collect())
}
simd_json::OwnedValue::Object(obj) => {
let map: serde_json::Map<String, serde_json::Value> = obj
.into_iter()
.map(|(k, v)| (k.into(), owned_value_into_serde_json(v)))
.collect();
serde_json::Value::Object(map)
}
}
}
Keep the existing borrow-based function for call sites that only have
a &OwnedValue. Swap opensearch_sink, http_sink, both
surrealdb_sink sites, and avro.rs to the consuming function
directly. Leave s3_sink and delta_sink for a follow-up, since they
need their callers restructured to own the payload first (see "Where
it occurs"). convert.rs already has an 11-case test module covering
every OwnedValue variant for the existing borrow-based function; the
new function's tests can mirror those directly rather than being
written from scratch. Should land as its own PR, not folded into an
unrelated sink's branch.
Validation
| Check |
Result |
| Code |
Confirmed at 4/6 call sites: caller already owns the value and drops it right after (see "Where it occurs"). |
| Object keys |
OwnedValue::Object = Box<HashMap<String, Value, ObjectHasher>>, plain String keys, no Cow/beef. .into() in the fix is a plain move. |
| Upstream |
No existing apache/iggy issue/PR found via GitHub's issue search API. |
Benchmark: standalone crate (serde_json 1.0.151, simd-json 0.18.1, release + lto), converting a pre-built Vec<OwnedValue> of
1000 messages x 200 batches, one ~1 KB document each (25 x 24-byte
string fields); only the convert-and-drop loop is timed, cloning
happens outside it:
|
per message |
per 1000-msg batch |
| current (borrow) |
~6.1-6.2 us |
~6.1-6.2 ms |
| proposed (consume) |
~3.3 us |
~3.3 ms |
Stable across 3 runs, dev machine (not a bench box, directional, not
a committed number). ~2.8 ms/batch is real but small next to the
network I/O (bulk request, upload, query) each sink does right after,
so it won't move end-to-end latency under normal load; matters more
under sustained high throughput or with the network stubbed out.
Contribution
Good first issue
Description
core/connectors/sdk/src/convert.rs::owned_value_to_serde_jsontakes&simd_json::OwnedValueand recursively allocates a freshStringfor every string leaf (
s.to_string()) and every object key(
k.to_string()) it visits:At most call sites the caller already owns the
OwnedValueoutrightand drops it immediately after converting. Converting between
OwnedValueandserde_json::Valuemeans rebuilding the tree eitherway, since they're unrelated types, but the leaf strings and object
keys don't need to ride along with that rebuild: they're already-owned
heap allocations one line away from being dropped, so
.to_string()copies their bytes into a new allocation just to throw the original
away right after.
Affected area / component
Connectors
Proposed solution
Add a consuming variant, additive (no breaking signature change):
Keep the existing borrow-based function for call sites that only have
a
&OwnedValue. Swapopensearch_sink,http_sink, bothsurrealdb_sinksites, andavro.rsto the consuming functiondirectly. Leave
s3_sinkanddelta_sinkfor a follow-up, since theyneed their callers restructured to own the payload first (see "Where
it occurs").
convert.rsalready has an 11-case test module coveringevery
OwnedValuevariant for the existing borrow-based function; thenew function's tests can mirror those directly rather than being
written from scratch. Should land as its own PR, not folded into an
unrelated sink's branch.
Validation
OwnedValue::Object=Box<HashMap<String, Value, ObjectHasher>>, plainStringkeys, noCow/beef..into()in the fix is a plain move.Benchmark: standalone crate (
serde_json 1.0.151,simd-json 0.18.1, release + lto), converting a pre-builtVec<OwnedValue>of1000 messages x 200 batches, one ~1 KB document each (25 x 24-byte
string fields); only the convert-and-drop loop is timed, cloning
happens outside it:
Stable across 3 runs, dev machine (not a bench box, directional, not
a committed number). ~2.8 ms/batch is real but small next to the
network I/O (bulk request, upload, query) each sink does right after,
so it won't move end-to-end latency under normal load; matters more
under sustained high throughput or with the network stubbed out.
Contribution
Good first issue