perf: pre-calculate buffer size in stalk walking

Author: morrisonlevi

profiling/Cargo.toml

@@ -47,6 +47,10 @@ perfcnt = "0.8.0"
 name = "stack_walking"
 harness = false
 
+[[bench]]
+name = "concatenation"
+harness = false
+
 [features]
 default = ["allocation_profiling", "timeline", "exception_profiling"]
 allocation_profiling = []

profiling/benches/concatenation.rs

@@ -0,0 +1,87 @@
+use criterion::{criterion_group, criterion_main, Criterion};
+
+#[allow(unused)]
+fn extract_function_name_v1(module_name: &[u8], class_name: &[u8], method_name: &[u8]) -> Vec<u8> {
+    let mut buffer = Vec::<u8>::new();
+
+    if !module_name.is_empty() {
+        buffer.extend_from_slice(module_name);
+        buffer.push(b'|');
+    }
+
+    if !class_name.is_empty() {
+        buffer.extend_from_slice(class_name);
+        buffer.extend_from_slice(b"::");
+    }
+    buffer.extend_from_slice(method_name);
+    buffer
+}
+
+#[allow(unused)]
+fn extract_function_name_v2(module_name: &[u8], class_name: &[u8], method_name: &[u8]) -> Vec<u8> {
+    let opt_module_separator: &[u8] = if module_name.is_empty() { b"" } else { b"|" };
+    let opt_class_separator: &[u8] = if class_name.is_empty() { b"" } else { b"::" };
+    let cap = module_name.len()
+        + opt_module_separator.len()
+        + class_name.len()
+        + opt_class_separator.len()
+        + method_name.len();
+    let mut buffer = Vec::<u8>::with_capacity(cap);
+
+    buffer.extend_from_slice(module_name);
+    buffer.extend_from_slice(opt_module_separator);
+    buffer.extend_from_slice(class_name);
+    buffer.extend_from_slice(opt_class_separator);
+    buffer.extend_from_slice(method_name);
+    buffer
+}
+
+fn bench_concatenation_userland(c: &mut Criterion) {
+    c.bench_function("bench_concatenation_userland", |b| {
+        b.iter(|| {
+            for _ in 1..=100 {
+                _ = std::hint::black_box(extract_function_name_v2(
+                    b"",
+                    b"Twig\\Template",
+                    b"displayWithErrorHandling",
+                ))
+            }
+        });
+    });
+}
+
+fn bench_concatenation_internal1(c: &mut Criterion) {
+    c.bench_function("bench_concatenation_internal1", |b| {
+        b.iter(|| {
+            for _ in 1..=100 {
+                _ = std::hint::black_box(extract_function_name_v2(
+                    b"dom",
+                    b"DOMDocumentFragment",
+                    b"__construct",
+                ))
+            }
+        });
+    });
+}
+
+fn bench_concatenation_internal2(c: &mut Criterion) {
+    c.bench_function("bench_concatenation_internal2", |b| {
+        b.iter(|| {
+            for _ in 1..=100 {
+                _ = std::hint::black_box(extract_function_name_v2(
+                    b"standard",
+                    b"",
+                    b"file",
+                ))
+            }
+        });
+    });
+}
+
+criterion_group!(
+    benches,
+    bench_concatenation_userland,
+    bench_concatenation_internal1,
+    bench_concatenation_internal2,
+);
+criterion_main!(benches);

profiling/src/bindings/mod.rs

@@ -694,4 +694,7 @@ mod tests {
             mem::align_of::<*mut usize>()
         );
     }
+
+    #[test]
+    fn test_hashmap_size_for_zend_extension() {}
 }

profiling/src/lib.rs

@@ -313,7 +313,7 @@ extern "C" fn minit(_type: c_int, module_number: c_int) -> ZendResult {
     };
 
     /* Currently, the engine is always copying this struct. Every time a new
-     * PHP version is released, we should double check zend_register_extension
+     * PHP version is released, we should double-check zend_register_extension
      * to ensure the address is not mutated nor stored. Well, hopefully we
      * catch it _before_ a release.
      */

profiling/src/profiling/stack_walking.rs

@@ -49,22 +49,27 @@ pub fn extract_function_name_into_buffer(func: &zend_function) -> Option<Vec<u8>
         return None;
     }
 
-    let mut buffer = Vec::<u8>::new();
-
     // User functions do not have a "module". Maybe one day use composer info?
     let module_name = func.module_name().unwrap_or(b"");
-    if !module_name.is_empty() {
-        buffer.extend_from_slice(module_name);
-        buffer.push(b'|');
-    }
-
     let class_name = func.scope_name().unwrap_or(b"");
-    if !class_name.is_empty() {
-        buffer.extend_from_slice(class_name);
-        buffer.extend_from_slice(b"::");
-    }
-
+    let opt_module_separator: &[u8] = if module_name.is_empty() { b"" } else { b"|" };
+    let opt_class_separator: &[u8] = if class_name.is_empty() { b"" } else { b"::" };
+
+    // Precalculating the allocation size and removing the conditionals was
+    // shown to be faster in criterion benchmarks.
+    let cap = module_name.len()
+        + opt_module_separator.len()
+        + class_name.len()
+        + opt_class_separator.len()
+        + method_name.len();
+    let mut buffer = Vec::<u8>::with_capacity(cap);
+
+    buffer.extend_from_slice(module_name);
+    buffer.extend_from_slice(opt_module_separator);
+    buffer.extend_from_slice(class_name);
+    buffer.extend_from_slice(opt_class_separator);
     buffer.extend_from_slice(method_name);
+
     Some(buffer)
 }