performance: optimized string and various other XDS improvements (ist…

…io#1016) * Initial * wip * prod compiling * bump max * wip diagnostics * performance: avoid clone of workload on conversion * performance: avoid allocating vector for all resources * intern: xds types * performance: optimize endpoint_uid * perf: do not allocate endpoints vector * performance: do not clone services map * use arcstr instead * metrics back on * Tests compile * format * fix and format * Move rbac over too * Move more over * rebase
MorrisLaw · May 3, 2024 · 5eb8906 · 5eb8906
1 parent a84ce37
commit 5eb8906
Show file tree

Hide file tree

Showing 35 changed files with 1,020 additions and 761 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -102,6 +102,8 @@ pingora-pool = "0.1.0"
 flurry = "0.5.0"
 h2 = "0.4"
 http = "1.1"
+split-iter = "0.1"
+arcstr = { version = "1.1", features = ["serde"] }
 
 [target.'cfg(target_os = "linux")'.dependencies]
 netns-rs = "0.1"

diff --git a/benches/throughput.rs b/benches/throughput.rs
@@ -39,7 +39,7 @@ use ztunnel::test_helpers::TEST_WORKLOAD_SOURCE;
 use ztunnel::test_helpers::TEST_WORKLOAD_TCP;
 use ztunnel::test_helpers::{helpers, tcp};
 use ztunnel::xds::LocalWorkload;
-use ztunnel::{app, identity, metrics, proxy, test_helpers};
+use ztunnel::{app, identity, metrics, proxy, strng, test_helpers};
 
 const KB: usize = 1024;
 const MB: usize = 1024 * KB;
@@ -68,15 +68,15 @@ fn create_test_policies() -> Vec<Authorization> {
     for _ in 0..N_RULES {
         rules.push(vec![vec![RbacMatch {
             namespaces: vec![
-                StringMatch::Prefix("random-prefix-2b123".to_string()),
-                StringMatch::Suffix("random-postix-2b723".to_string()),
-                StringMatch::Exact("random-exac-2bc13".to_string()),
+                StringMatch::Prefix("random-prefix-2b123".into()),
+                StringMatch::Suffix("random-postix-2b723".into()),
+                StringMatch::Exact("random-exac-2bc13".into()),
             ],
             not_namespaces: vec![],
             principals: vec![
-                StringMatch::Prefix("random-prefix-2b123".to_string()),
-                StringMatch::Suffix("random-postix-2b723".to_string()),
-                StringMatch::Exact("random-exac-2bc13".to_string()),
+                StringMatch::Prefix("random-prefix-2b123".into()),
+                StringMatch::Suffix("random-postix-2b723".into()),
+                StringMatch::Exact("random-exac-2bc13".into()),
             ],
             not_principals: vec![],
             source_ips: vec![DUMMY_NETWORK.parse().unwrap()],
@@ -90,10 +90,10 @@ fn create_test_policies() -> Vec<Authorization> {
 
     for i in 0..N_POLICIES {
         policies.push(Authorization {
-            name: format!("policy {i}"),
+            name: strng::format!("policy {i}"),
             action: ztunnel::rbac::RbacAction::Deny,
             scope: ztunnel::rbac::RbacScope::Global,
-            namespace: "default".to_string(),
+            namespace: "default".into(),
             rules: rules.clone(),
         });
     }
@@ -412,10 +412,10 @@ fn hbone_connection_config() -> ztunnel::config::ConfigSource {
             workload: Workload {
                 workload_ips: vec![hbone_connection_ip(i)],
                 protocol: Protocol::HBONE,
-                uid: format!("cluster1//v1/Pod/default/local-source{}", i),
-                name: format!("workload-{}", i),
-                namespace: format!("namespace-{}", i),
-                service_account: format!("service-account-{}", i),
+                uid: strng::format!("cluster1//v1/Pod/default/local-source{}", i),
+                name: strng::format!("workload-{}", i),
+                namespace: strng::format!("namespace-{}", i),
+                service_account: strng::format!("service-account-{}", i),
                 ..test_helpers::test_default_workload()
             },
             services: Default::default(),

diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock
diff --git a/fuzz/fuzz_targets/protobuf.rs b/fuzz/fuzz_targets/protobuf.rs
@@ -27,11 +27,11 @@ fuzz_target!(|data: &[u8]| {
 });
 
 fn run_workload(data: &[u8]) -> anyhow::Result<()> {
-    Workload::try_from(&XdsWorkload::decode(data)?)?;
+    Workload::try_from(XdsWorkload::decode(data)?)?;
     Ok(())
 }
 
 fn run_rbac(data: &[u8]) -> anyhow::Result<()> {
-    Authorization::try_from(&XdsAuthorization::decode(data)?)?;
+    Authorization::try_from(XdsAuthorization::decode(data)?)?;
     Ok(())
 }
diff --git a/src/admin.rs b/src/admin.rs
@@ -457,6 +457,7 @@ mod tests {
     use crate::config::construct_config;
     use crate::config::ProxyConfig;
     use crate::identity;
+    use crate::strng;
     use crate::test_helpers::{get_response_str, helpers, new_proxy_state};
     use crate::xds::istio::security::string_match::MatchType as XdsMatchType;
     use crate::xds::istio::security::Address as XdsAddress;
@@ -519,9 +520,9 @@ mod tests {
         for i in 0..2 {
             manager
                 .fetch_certificate(&identity::Identity::Spiffe {
-                    trust_domain: "trust_domain".to_string(),
-                    namespace: "namespace".to_string(),
-                    service_account: format!("sa-{i}"),
+                    trust_domain: "trust_domain".into(),
+                    namespace: "namespace".into(),
+                    service_account: strng::format!("sa-{i}"),
                 })
                 .await
                 .unwrap();

diff --git a/src/baggage.rs b/src/baggage.rs
@@ -12,18 +12,19 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use crate::strng::Strng;
 use hyper::{
     header::{GetAll, ToStrError},
     http::HeaderValue,
 };
 
 #[derive(Default)]
 pub struct Baggage {
-    pub cluster_id: Option<String>,
-    pub namespace: Option<String>,
-    pub workload_name: Option<String>,
-    pub service_name: Option<String>,
-    pub revision: Option<String>,
+    pub cluster_id: Option<Strng>,
+    pub namespace: Option<Strng>,
+    pub workload_name: Option<Strng>,
+    pub service_name: Option<Strng>,
+    pub revision: Option<Strng>,
 }
 
 pub fn parse_baggage_header(headers: GetAll<HeaderValue>) -> Result<Baggage, ToStrError> {
@@ -37,7 +38,7 @@ pub fn parse_baggage_header(headers: GetAll<HeaderValue>) -> Result<Baggage, ToS
             if parts.len() > 1 {
                 let val = match parts[1] {
                     "" => None,
-                    s => Some(s.to_string()),
+                    s => Some(s.into()),
                 };
                 match parts[0] {
                     "k8s.cluster.name" => baggage.cluster_id = val,
@@ -71,11 +72,11 @@ pub mod tests {
         let header_value = HeaderValue::from_str(baggage_str)?;
         hm.append(BAGGAGE_HEADER, header_value);
         let baggage = parse_baggage_header(hm.get_all(BAGGAGE_HEADER))?;
-        assert_eq!(baggage.cluster_id, Some("K1".to_string()));
-        assert_eq!(baggage.namespace, Some("NS1".to_string()));
-        assert_eq!(baggage.workload_name, Some("N1".to_string()));
-        assert_eq!(baggage.service_name, Some("N2".to_string()));
-        assert_eq!(baggage.revision, Some("V1".to_string()));
+        assert_eq!(baggage.cluster_id, Some("K1".into()));
+        assert_eq!(baggage.namespace, Some("NS1".into()));
+        assert_eq!(baggage.workload_name, Some("N1".into()));
+        assert_eq!(baggage.service_name, Some("N2".into()));
+        assert_eq!(baggage.revision, Some("V1".into()));
         Ok(())
     }
 
@@ -112,11 +113,11 @@ pub mod tests {
         hm.append(BAGGAGE_HEADER, HeaderValue::from_str("service.name=N2")?);
         hm.append(BAGGAGE_HEADER, HeaderValue::from_str("service.version=V1")?);
         let baggage = parse_baggage_header(hm.get_all(BAGGAGE_HEADER))?;
-        assert_eq!(baggage.cluster_id, Some("K1".to_string()));
-        assert_eq!(baggage.namespace, Some("NS1".to_string()));
-        assert_eq!(baggage.workload_name, Some("N1".to_string()));
-        assert_eq!(baggage.service_name, Some("N2".to_string()));
-        assert_eq!(baggage.revision, Some("V1".to_string()));
+        assert_eq!(baggage.cluster_id, Some("K1".into()));
+        assert_eq!(baggage.namespace, Some("NS1".into()));
+        assert_eq!(baggage.workload_name, Some("N1".into()));
+        assert_eq!(baggage.service_name, Some("N2".into()));
+        assert_eq!(baggage.revision, Some("V1".into()));
         Ok(())
     }
 

diff --git a/src/cert_fetcher.rs b/src/cert_fetcher.rs
@@ -94,7 +94,7 @@ impl CertFetcherImpl {
         // Only shared mode fetches other workloads's certs
         self.proxy_mode == ProxyMode::Shared &&
             // We only get certs for our own node
-            Some(&w.node) == self.local_node.as_ref() &&
+            Some(w.node.as_ref()) == self.local_node.as_deref() &&
             // If it doesn't support HBONE it *probably* doesn't need a cert.
             (w.native_tunnel || w.protocol == Protocol::HBONE)
     }

diff --git a/src/config.rs b/src/config.rs
@@ -27,6 +27,7 @@ use hyper::http::uri::InvalidUri;
 use hyper::Uri;
 
 use crate::identity;
+use crate::strng::Strng;
 #[cfg(any(test, feature = "testing"))]
 use {crate::test_helpers::MpscAckReceiver, crate::xds::LocalConfig, tokio::sync::Mutex};
 
@@ -155,7 +156,7 @@ pub struct Config {
     pub dns_proxy_addr: SocketAddr,
 
     /// The network of the node this ztunnel is running on.
-    pub network: String,
+    pub network: Strng,
     /// The name of the node this ztunnel is running as.
     pub local_node: Option<String>,
     /// The proxy mode of ztunnel, Shared or Dedicated, default to Shared.

diff --git a/src/dns/metrics.rs b/src/dns/metrics.rs
@@ -23,6 +23,7 @@ use std::time::Duration;
 use crate::metrics::{DefaultedUnknown, DeferRecorder, Recorder};
 
 use crate::state::workload::Workload;
+use crate::strng::RichStrng;
 
 pub struct Metrics {
     pub requests: Family<DnsLabels, Counter>,
@@ -77,19 +78,19 @@ impl DeferRecorder for Metrics {}
 
 #[derive(Clone, Hash, Debug, PartialEq, Eq, EncodeLabelSet)]
 pub struct DnsLabels {
-    request_query_type: String,
-    request_protocol: String,
+    request_query_type: RichStrng,
+    request_protocol: RichStrng,
 
     // Source workload.
-    source_canonical_service: DefaultedUnknown<String>,
-    source_canonical_revision: DefaultedUnknown<String>,
+    source_canonical_service: DefaultedUnknown<RichStrng>,
+    source_canonical_revision: DefaultedUnknown<RichStrng>,
 }
 
 impl DnsLabels {
     pub fn new(r: &Request) -> Self {
         Self {
-            request_query_type: r.query().query_type().to_string().to_lowercase(),
-            request_protocol: r.protocol().to_string().to_lowercase(),
+            request_query_type: r.query().query_type().to_string().to_lowercase().into(),
+            request_protocol: r.protocol().to_string().to_lowercase().into(),
             source_canonical_service: Default::default(),
             source_canonical_revision: Default::default(),
         }