search

interledger / interledger-rs

An easy-to-use, high-performance Interledger implementation written in Rust
http://interledger.rs
Other
201 stars 70 forks source link

Don't clone the entire service chain on every packet #615

Open kincaidoneil opened 4 years ago

kincaidoneil commented 4 years ago

Since calling handle_request on the service interface requires a mutable borrow, we're cloning the entire service chain on every packet to get around this.

For ILP-over-HTTP requests, it happens twice:

https://github.com/interledger-rs/interledger-rs/blob/4b491ab76ca4b73ef53e101fc2cb1a71e32f53d2/crates/interledger-http/src/server.rs#L124-L135

https://github.com/interledger-rs/interledger-rs/blob/4b491ab76ca4b73ef53e101fc2cb1a71e32f53d2/crates/interledger-http/src/server.rs#L63-L74

Here's where it happens for BTP packets:

https://github.com/interledger-rs/interledger-rs/blob/4b491ab76ca4b73ef53e101fc2cb1a71e32f53d2/crates/interledger-btp/src/service.rs#L258-L259

This probably isn't great for memory usage or performance (and is probably much worse than #469 or #554). We should consider redesigning the service interface so it doesn't require a mutable borrow, since it appears very few (if any?) services require mutation.

gakonst commented 4 years ago

It's not the mutable borrow that makes us require the clone, it's the requirement of the closure to be Fn. If you don't clone and pass by value it'll obviously be FnOnce, which doesn't work. Alternatively, you could try adding references to work around this, but I haven't figured out how to make lifetimes work in this case. There might be a way to get it working, but do note that even the Warp docs capture the state with a clone.