Conversation
…ation # Conflicts: # Cargo.toml
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #121 +/- ##
==========================================
- Coverage 59.89% 58.61% -1.28%
==========================================
Files 82 84 +2
Lines 4266 4311 +45
==========================================
- Hits 2555 2527 -28
- Misses 1711 1784 +73
☔ View full report in Codecov by Sentry. |
krisbitney
requested review from
Niraj-Kamdar,
cbrzn,
dOrgJelli,
namesty,
nerfZael and
pileks
and removed request for
cbrzn and
nerfZael
…ation # Conflicts: # packages/default-config/Cargo.toml
…ation # Conflicts: # Cargo.toml # packages/builder/src/helpers.rs # packages/client/tests/test_map_type.rs
…ation # Conflicts: # Cargo.toml
| let http_uri = UriCase { | ||
| id: "http_uri".to_string(), | ||
| uri: Uri::try_from(format!( | ||
| "http/https://raw.githubusercontent.com/polywrap/rust-client/kris/debug-slow-invocation/packages/benchmarks/fibonacci/rs/build" |
There was a problem hiding this comment.
I think we should create a benchmark-wraps repo in the polywrap organization and use that
There was a problem hiding this comment.
yeah, sounds good! I'll do that soon.
There was a problem hiding this comment.
to clarify, are you suggesting we no longer benchmark fs resolution, or that we should write a script to clone the benchmark-wraps repo temporarily in order to test fs resolution?
|
|
||
| pub fn get_fibonacci_wrap(implementation: &str) -> WasmWrapper { | ||
| let path = Path::new(&get_fibonacci_dir(implementation)).join("wrap.wasm"); | ||
| let module_bytes = fs::read(path).unwrap(); |
There was a problem hiding this comment.
You can use include_bytes! for this instead of the file system to get some performence increase. Check out how it's done in the default-config crate.
Note: you wont be able to pass in the implementation str, but that's an ok compromise IMO
There was a problem hiding this comment.
That's awesome. This isn't affecting the benchmarks. Do you think the change still needs to be made?
There was a problem hiding this comment.
I think so, if it's not too much trouble. It makes sense cause it decreases time it takes for the tests to run (so developers can confirm their code works faster, CI is faster, etc). Ideally, we'd use include_bytes everywhere instead of fs::read everywhere. Ofc, one-off uses don't affect much :D
2 participants
This PR adds a benchmark package to measure the performance of various client methods