proxy-wasm-rust-sdk icon indicating copy to clipboard operation
proxy-wasm-rust-sdk copied to clipboard

Get property without Vec allocation

Open andresmargalef opened this issue 2 years ago • 4 comments

This patch add a new struct PropertyPath and a macro used to create a PropertyPath with a slice of &str using const generics to prevent allocations in runtime.

Before:

let property_path = vec!["path", "to", "property"];
let property_bytes = match proxy_wasm::hostcalls::get_property(property_path) {
    Ok(value) => value,
    _ => return Action::Continue,
};

New method:

let property_path = create_property_path!("path", "to", "property");
let property_bytes = match proxy_wasm::hostcalls::get_property_by_path(&property_path) {
    Ok(value) => value,
    _ => return Action::Continue,
};

andresmargalef avatar Nov 03 '23 20:11 andresmargalef

@andresmargalef this looks great, but I'm wondering what's the motivation behind this change? Does this result in measurable performance changes in your plugins?

PiotrSikora avatar Nov 09 '23 17:11 PiotrSikora

The motivation is latency reduction removing extra work like allocations, right now i'm building an example application using wasmtime to test locally ours filters using this patch vs master. I will share any results here :D

andresmargalef avatar Nov 09 '23 19:11 andresmargalef

The latency reduction is ~30%, the "toy" benchmark runs 1000 iterations and calculate the avg in nanos. The test call only one get_property using a vec vs get_property_path, and on the host side always returns 1 (NotFound). I need to improve de code and use criterion to get better numbers.

request_metrics.wasm.optimized[1000 iterations]: 452 avg nanos
request_metrics.wasm.slow[1000 iterations]: 510 avg nanos
request_metrics.wasm.optimized[1000 iterations]: 278 avg nanos
request_metrics.wasm.slow[1000 iterations]: 634 avg nanos
request_metrics.wasm.optimized[1000 iterations]: 230 avg nanos
request_metrics.wasm.slow[1000 iterations]: 426 avg nanos
request_metrics.wasm.optimized[1000 iterations]: 196 avg nanos
request_metrics.wasm.slow[1000 iterations]: 278 avg nanos
request_metrics.wasm.optimized[1000 iterations]: 191 avg nanos
request_metrics.wasm.slow[1000 iterations]: 289 avg nanos
request_metrics.wasm.optimized[1000 iterations]: 191 avg nanos
request_metrics.wasm.slow[1000 iterations]: 276 avg nanos
request_metrics.wasm.optimized[1000 iterations]: 188 avg nanos
request_metrics.wasm.slow[1000 iterations]: 276 avg nanos
request_metrics.wasm.optimized[1000 iterations]: 188 avg nanos
request_metrics.wasm.slow[1000 iterations]: 277 avg nanos

andresmargalef avatar Nov 10 '23 02:11 andresmargalef

Hi @PiotrSikora , do you think it makes sense to merge this pull request? Do not hesitate to tell me about any changes you see that need to be made as well as any other perf test.

andresmargalef avatar Nov 16 '23 12:11 andresmargalef