diff --git a/profiling/Cargo.toml b/profiling/Cargo.toml index edc1219d681..59b94e06e6c 100644 --- a/profiling/Cargo.toml +++ b/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 = [] diff --git a/profiling/benches/concatenation.rs b/profiling/benches/concatenation.rs new file mode 100644 index 00000000000..96239e2576b --- /dev/null +++ b/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 { + let mut buffer = Vec::::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 { + 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::::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); diff --git a/profiling/src/bindings/mod.rs b/profiling/src/bindings/mod.rs index 8258ec3da6b..35ac4b02d3a 100644 --- a/profiling/src/bindings/mod.rs +++ b/profiling/src/bindings/mod.rs @@ -694,4 +694,7 @@ mod tests { mem::align_of::<*mut usize>() ); } + + #[test] + fn test_hashmap_size_for_zend_extension() {} } diff --git a/profiling/src/lib.rs b/profiling/src/lib.rs index 61b3ab62e2e..b663a670a7b 100644 --- a/profiling/src/lib.rs +++ b/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. */ diff --git a/profiling/src/profiling/stack_walking.rs b/profiling/src/profiling/stack_walking.rs index 2564460f060..dfcd68f4059 100644 --- a/profiling/src/profiling/stack_walking.rs +++ b/profiling/src/profiling/stack_walking.rs @@ -49,22 +49,27 @@ pub fn extract_function_name_into_buffer(func: &zend_function) -> Option return None; } - let mut buffer = Vec::::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::::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) }