Skip to content

Commit 93eb621

Browse files
authored
perf(datapath): eliminate allocations from publish routing hot path (agntcy#1577)
# Description The publish routing hot path allocates unnecessarily on every inbound publish message. `SlimHeader::get_dst()` converts the proto `Name` into an internal `Name`, allocating `Arc<[String; 3]>` and cloning 3 strings from the wire message. The routing path (subscription table matching) never uses these strings — it only needs the four `u64` hash components already present as `EncodedName` in the proto. This PR threads a zero-allocation `EncodedName` (a `Copy` struct of 4 `u64`s) through the entire publish routing path, eliminating all heap allocation from the hot path. **Benchmark results (Apple M-series, release profile):** | Benchmark | Time | Change | |---|---|---| | `routing_via_name` (before) | ~108 ns | baseline | | `routing_via_encoded_name` (after) | ~38 ns | **~65% faster** | Closes agntcy#1386 ## Changes **Phase 1 — Benchmark baseline (separate commit)** - Add criterion benchmark suite covering `Name` construction and the full publish routing loop (`routing_via_name` baseline) **Phase 2 — Code changes** - Add `SlimHeader::get_encoded_dst()` / `get_encoded_source()` returning `EncodedName` (Copy, zero allocation) - Add `From<&Name> for EncodedName` conversion - Replace `SubscriptionTable::match_one` / `match_all` to accept `&EncodedName` instead of `&Name` - Replace `InternalName([u64; 4])` with `SubscriptionName([u64; 3])`, eliminating unsafe `transmute` tricks and the redundant 4th component (the id is never part of the subscription group key) - Replace `representative_name: Name` in `NameState` with `representative_strings: [String; 3]` — the human-readable name this `NameState` represents in the subscription table — reconstructing a `Name` only in the cold `for_each` path - Simplify `process_publish` and `match_and_forward_msg`; retain `get_dst()` call in the debug log for human-readable output - Add `DataPathError::NoMatchEncoded` for the allocation-free error path ## Type of Change - [ ] Bugfix - [x] New Feature - [ ] Breaking Change - [x] Refactor - [ ] Documentation - [ ] Other (please describe) ## Checklist - [ ] I have read the [contributing guidelines](/agntcy/repo-template/blob/main/CONTRIBUTING.md) - [x] Existing issues have been referenced (where applicable) - [x] I have verified this change is not present in other open pull requests - [ ] Functionality is documented - [x] All code style checks pass - [x] New code contribution is covered by automated tests - [x] All new and existing tests pass --------- Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
1 parent 9d09cfe commit 93eb621

10 files changed

Lines changed: 408 additions & 141 deletions

File tree

data-plane/core/datapath/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,7 @@ tracing-test = { workspace = true, features = ["no-env-filter"] }
4949
[[bench]]
5050
name = "pool_benchmark"
5151
harness = false
52+
53+
[[bench]]
54+
name = "name_benchmark"
55+
harness = false
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
// Copyright AGNTCY Contributors (https://github.com/agntcy)
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
use criterion::{Criterion, black_box, criterion_group, criterion_main};
5+
use slim_datapath::api::{EncodedName, ProtoName, SlimHeader, StringName};
6+
use slim_datapath::messages::encoder::Name;
7+
use slim_datapath::tables::SubscriptionTable;
8+
use slim_datapath::tables::subscription_table::SubscriptionTableImpl;
9+
10+
fn make_proto_name() -> ProtoName {
11+
ProtoName {
12+
name: Some(EncodedName {
13+
component_0: 0x1234_5678_9abc_def0,
14+
component_1: 0xfeed_cafe_dead_beef,
15+
component_2: 0x0101_0101_0101_0101,
16+
component_3: u64::MAX,
17+
}),
18+
str_name: Some(StringName {
19+
str_component_0: "org".to_string(),
20+
str_component_1: "namespace".to_string(),
21+
str_component_2: "agent".to_string(),
22+
}),
23+
}
24+
}
25+
26+
/// Build a SlimHeader whose destination matches the subscription used in routing benchmarks
27+
/// (org/namespace/agent, id=42). Hashes are derived from Name::from_strings to ensure they
28+
/// match whatever SubscriptionTableImpl stores.
29+
fn make_slim_header() -> SlimHeader {
30+
let name = Name::from_strings(["org", "namespace", "agent"]).with_id(42);
31+
let c = name.components();
32+
let proto_name = ProtoName {
33+
name: Some(EncodedName {
34+
component_0: c[0],
35+
component_1: c[1],
36+
component_2: c[2],
37+
component_3: c[3],
38+
}),
39+
str_name: Some(StringName {
40+
str_component_0: "org".to_string(),
41+
str_component_1: "namespace".to_string(),
42+
str_component_2: "agent".to_string(),
43+
}),
44+
};
45+
SlimHeader {
46+
source: Some(proto_name.clone()),
47+
destination: Some(proto_name),
48+
identity: String::new(),
49+
fanout: 1,
50+
recv_from: None,
51+
forward_to: None,
52+
incoming_conn: None,
53+
error: None,
54+
}
55+
}
56+
57+
fn bench_name_from_strings(c: &mut Criterion) {
58+
c.bench_function("name_from_strings", |b| {
59+
b.iter(|| black_box(Name::from_strings(black_box(["org", "namespace", "agent"]))))
60+
});
61+
}
62+
63+
fn bench_name_from_proto(c: &mut Criterion) {
64+
let proto_name = make_proto_name();
65+
c.bench_function("name_from_proto", |b| {
66+
b.iter(|| black_box(Name::from(black_box(&proto_name))))
67+
});
68+
}
69+
70+
fn bench_name_clone_with_strings(c: &mut Criterion) {
71+
let name = Name::from_strings(["org", "namespace", "agent"]);
72+
c.bench_function("name_clone_with_strings", |b| {
73+
b.iter(|| black_box(name.clone()))
74+
});
75+
}
76+
77+
/// Baseline: full routing flow via the Name-based path.
78+
/// get_dst() allocates Arc<[String; 3]> + clones 3 Strings.
79+
fn bench_routing_via_name(c: &mut Criterion) {
80+
let table = SubscriptionTableImpl::default();
81+
let sub = Name::from_strings(["org", "namespace", "agent"]).with_id(42);
82+
table.add_subscription(sub, 1, true, 1).unwrap();
83+
let slim_header = make_slim_header();
84+
85+
c.bench_function("routing_via_name", |b| {
86+
b.iter(|| {
87+
let dst = black_box(slim_header.get_dst());
88+
black_box(table.match_one(black_box(&EncodedName::from(&dst)), 0))
89+
})
90+
});
91+
}
92+
93+
/// Optimized: full routing flow via the EncodedName path.
94+
/// get_encoded_dst() is a Copy of 4 u64s — zero heap allocation.
95+
fn bench_routing_via_encoded_name(c: &mut Criterion) {
96+
let table = SubscriptionTableImpl::default();
97+
let sub = Name::from_strings(["org", "namespace", "agent"]).with_id(42);
98+
table.add_subscription(sub, 1, true, 1).unwrap();
99+
let slim_header = make_slim_header();
100+
101+
c.bench_function("routing_via_encoded_name", |b| {
102+
b.iter(|| {
103+
let encoded = black_box(slim_header.get_encoded_dst());
104+
black_box(table.match_one(black_box(&encoded), 0))
105+
})
106+
});
107+
}
108+
109+
criterion_group!(
110+
benches,
111+
bench_name_from_strings,
112+
bench_name_from_proto,
113+
bench_name_clone_with_strings,
114+
bench_routing_via_name,
115+
bench_routing_via_encoded_name,
116+
);
117+
118+
criterion_main!(benches);

data-plane/core/datapath/src/api.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ pub use proto::dataplane::v1::CommandPayload;
99
pub use proto::dataplane::v1::Content;
1010
pub use proto::dataplane::v1::DiscoveryReplyPayload;
1111
pub use proto::dataplane::v1::DiscoveryRequestPayload;
12+
pub use proto::dataplane::v1::EncodedName;
1213
pub use proto::dataplane::v1::GroupAckPayload;
1314
pub use proto::dataplane::v1::GroupAddPayload;
1415
pub use proto::dataplane::v1::GroupNackPayload;
@@ -29,6 +30,7 @@ pub use proto::dataplane::v1::SessionHeader;
2930
pub use proto::dataplane::v1::SessionMessageType as ProtoSessionMessageType;
3031
pub use proto::dataplane::v1::SessionType as ProtoSessionType;
3132
pub use proto::dataplane::v1::SlimHeader;
33+
pub use proto::dataplane::v1::StringName;
3234
pub use proto::dataplane::v1::Subscribe as ProtoSubscribe;
3335
pub use proto::dataplane::v1::SubscriptionAck as ProtoSubscriptionAck;
3436
pub use proto::dataplane::v1::Unsubscribe as ProtoUnsubscribe;

data-plane/core/datapath/src/errors.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ pub enum DataPathError {
2727
// Subscription / matching
2828
#[error("no matching found for {0}")]
2929
NoMatch(Name),
30+
#[error("no matching found for [{:x}, {:x}, {:x}, {:x}]", .0[0], .0[1], .0[2], .0[3])]
31+
NoMatchEncoded([u64; 4]),
3032
#[error("subscription not found")]
3133
SubscriptionNotFound(Name),
3234
#[error("subscription id not found: {0}")]

data-plane/core/datapath/src/forwarder.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use super::tables::SubscriptionTable;
88
use super::tables::connection_table::ConnectionTable;
99
use super::tables::remote_subscription_table::RemoteSubscriptions;
1010
use super::tables::subscription_table::SubscriptionTableImpl;
11+
use crate::api::EncodedName;
1112
use crate::errors::DataPathError;
1213
use crate::messages::Name;
1314
use crate::tables::remote_subscription_table::SubscriptionInfo;
@@ -129,16 +130,16 @@ where
129130

130131
pub fn on_publish_msg_match(
131132
&self,
132-
name: Name,
133+
encoded: EncodedName,
133134
incoming_conn: u64,
134135
fanout: u32,
135136
) -> Result<Vec<u64>, DataPathError> {
136137
if fanout == 1 {
137138
self.subscription_table
138-
.match_one(&name, incoming_conn)
139+
.match_one(&encoded, incoming_conn)
139140
.map(|out| vec![out])
140141
} else {
141-
self.subscription_table.match_all(&name, incoming_conn)
142+
self.subscription_table.match_all(&encoded, incoming_conn)
142143
}
143144
}
144145

@@ -177,15 +178,15 @@ mod tests {
177178
);
178179

179180
assert_eq!(
180-
fwd.on_publish_msg_match(name.clone().with_id(1), 100, 1)
181+
fwd.on_publish_msg_match(EncodedName::from(&name.clone().with_id(1)), 100, 1)
181182
.unwrap(),
182183
vec![12]
183184
);
184185

185186
let expected = name.clone().with_id(2);
186187

187-
let err = fwd.on_publish_msg_match(expected.clone(), 100, 1);
188-
assert!(matches!(err, Err(DataPathError::NoMatch(_))));
188+
let err = fwd.on_publish_msg_match(EncodedName::from(&expected), 100, 1);
189+
assert!(matches!(err, Err(DataPathError::NoMatchEncoded(_))));
189190

190191
assert!(
191192
fwd.on_subscription_msg(name.clone(), 10, false, false, 1)

data-plane/core/datapath/src/message_processing.rs

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use crate::api::ProtoSubscriptionAckType as SubscriptionAckType;
3030
use crate::api::ProtoUnsubscribeType as UnsubscribeType;
3131
use crate::api::proto::dataplane::v1::Message;
3232
use crate::api::{
33-
LinkNegotiationPayload, ProtoLink, ProtoLinkMessageType as LinkType, ProtoLinkType,
33+
EncodedName, LinkNegotiationPayload, ProtoLink, ProtoLinkMessageType as LinkType, ProtoLinkType,
3434
};
3535
use semver;
3636

@@ -435,15 +435,11 @@ impl MessageProcessor {
435435
&self,
436436
#[cfg(feature = "otel_tracing")] mut msg: Message,
437437
#[cfg(not(feature = "otel_tracing"))] msg: Message,
438-
name: Name,
439438
in_connection: u64,
440439
fanout: u32,
441440
) -> Result<(), DataPathError> {
442-
debug!(
443-
%name,
444-
%fanout,
445-
"match and forward message"
446-
);
441+
let header = msg.get_slim_header();
442+
debug!(name = %header.get_dst(), %fanout, "match and forward message");
447443

448444
// if the message already contains an output connection, use that one
449445
// without performing any match in the subscription table
@@ -452,9 +448,11 @@ impl MessageProcessor {
452448
return self.send_msg(msg, val).await;
453449
}
454450

451+
let encoded = header.get_encoded_dst();
452+
455453
match self
456454
.forwarder()
457-
.on_publish_msg_match(name, in_connection, fanout)
455+
.on_publish_msg_match(encoded, in_connection, fanout)
458456
{
459457
Ok(out_vec) => {
460458
let len = out_vec.len();
@@ -639,17 +637,11 @@ impl MessageProcessor {
639637
);
640638
//////////////////////////////////////////////////////
641639

642-
// get header
643-
let header = msg.get_slim_header();
644-
645-
let dst = header.get_dst();
646-
647640
// this function may panic, but at this point we are sure we are processing
648641
// a publish message
649642
let fanout = msg.get_fanout();
650643

651-
self.match_and_forward_msg(msg, dst, in_connection, fanout)
652-
.await
644+
self.match_and_forward_msg(msg, in_connection, fanout).await
653645
}
654646

655647
pub(crate) async fn send_subscription_ack(
@@ -1227,7 +1219,7 @@ impl MessageProcessor {
12271219
.into_iter()
12281220
.filter(|(name, _)| {
12291221
mp.forwarder()
1230-
.on_publish_msg_match(name.clone(), u64::MAX, u32::MAX)
1222+
.on_publish_msg_match(EncodedName::from(name), u64::MAX, u32::MAX)
12311223
.is_err()
12321224
})
12331225
.collect();
@@ -1345,6 +1337,7 @@ mod tests {
13451337

13461338
use super::*;
13471339
use crate::api::ProtoSubscriptionAck;
1340+
use crate::messages::Name;
13481341
use crate::tables::remote_subscription_table::SubscriptionInfo;
13491342
use tonic::Status;
13501343

@@ -2298,9 +2291,10 @@ mod tests {
22982291
.unwrap();
22992292

23002293
// The subscription should have been restored in the routing table.
2301-
let result = processor
2302-
.forwarder()
2303-
.on_publish_msg_match(sub_name, u64::MAX, 1);
2294+
let result =
2295+
processor
2296+
.forwarder()
2297+
.on_publish_msg_match(EncodedName::from(&sub_name), u64::MAX, 1);
23042298
assert!(result.is_ok(), "recovered subscription should be routable");
23052299
assert_eq!(result.unwrap(), vec![conn_id]);
23062300
}

data-plane/core/datapath/src/messages/encoder.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::hash::{Hash, Hasher};
55
use std::sync::Arc;
66
use twox_hash::XxHash64;
77

8-
use crate::api::ProtoName;
8+
use crate::api::{EncodedName, ProtoName};
99

1010
#[derive(Clone)]
1111
pub struct Name {
@@ -82,6 +82,18 @@ impl From<&ProtoName> for Name {
8282
}
8383
}
8484

85+
impl From<&Name> for EncodedName {
86+
fn from(name: &Name) -> Self {
87+
let c = name.components();
88+
EncodedName {
89+
component_0: c[0],
90+
component_1: c[1],
91+
component_2: c[2],
92+
component_3: c[3],
93+
}
94+
}
95+
}
96+
8597
impl Name {
8698
// NULL_COMPONENT is used to represent a component that is not set
8799
pub const NULL_COMPONENT: u64 = u64::MAX;

0 commit comments

Comments
 (0)