warp icon indicating copy to clipboard operation
warp copied to clipboard

perf: Reduce the amount of code generated for path!

Open Marwes opened this issue 4 years ago • 10 comments

This does two things to reduce the amount of code path! generates, first it removes warp::any() filter at the start of each path, therefore removing one level of And as well.

Next I modified the macro to accumulate consecutive string literal segments and combining them into one via concat!. Since each segment generates its own static type this significantly reduce the amount of code when this applies (path!("abc" / "123" / "456")). I did have to cahge the behaviour of path() for this to accept / but I can change this combining to just be an implementation detail instead.

Marwes avatar Oct 14 '20 13:10 Marwes

Any chance of this getting merged?

Marwes avatar Dec 08 '20 15:12 Marwes

Thanks for the PR! I'd prefer to keep the requirement that exact not allow a string with a slash. What did you have in mind for "I can change this combining to just be an implementation detail instead"?

seanmonstar avatar Dec 09 '20 00:12 seanmonstar

I'd just add a #[doc(hidden)] variant of the function which is used by the macro

Marwes avatar Dec 09 '20 10:12 Marwes

Rebased with the fix to path

Marwes avatar Dec 22 '20 14:12 Marwes

Any further issues with this? The assert is restored and I added a hidden __path function which the macro calls into which still allows / in the path.

Marwes avatar Jan 12 '21 14:01 Marwes

@seanmonstar Fixed and added a test

Marwes avatar Feb 05 '21 16:02 Marwes

I see a new label waiting-on-author was added.

I don't think this PR is waiting on the Author (@Marwes) but rather it is waiting for the maintainer @seanmonstar to review.

ramn avatar Jun 11 '21 13:06 ramn

ah righ sorry, as I saw a question still unresolved I thought it wasn't yet addressed. I will try to review this PR this weekend 👍

jxs avatar Jun 11 '21 23:06 jxs

LGTM, only some minor nit :)

jxs avatar Jun 21 '21 22:06 jxs

Resurrecting this since I am back on project that uses warp. The PR as is seemed to have been mostly accepted (?) but I rebased it (no conflicts) and added the last commit to make the assertion a bit clearer.

Example llvm-lines/(re)compile time for a crate which uses warp::path! quite heavily.

Before

cargo llvm-lines --lib | head -50
   Compiling warp v0.3.3 (/Users/markus.westerlind/Code/warp)
   Compiling a-server v0.1.0 (/Users/markus.westerlind/Code/a/a-server)
    Finished dev [unoptimized + debuginfo] target(s) in 1m 22s
warning: the following packages contain code that will be rejected by a future version of Rust: buf_redux v0.8.4, multipart v0.18.0
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 1`
  Lines                  Copies               Function name
  -----                  ------               -------------
  1359048                40649                (TOTAL)
   106985 (7.9%,  7.9%)    521 (1.3%,  1.3%)  <warp::filter::and::State<T,TE,U> as core::future::future::Future>::poll
    33276 (2.4%, 10.3%)    708 (1.7%,  3.0%)  core::pin::Pin<P>::set
    32850 (2.4%, 12.7%)    146 (0.4%,  3.4%)  <warp::filters::path::Exact<P> as warp::filter::FilterBase>::filter::{{closure}}::{{closure}}
    22330 (1.6%, 14.4%)    203 (0.5%,  3.9%)  std::thread::local::LocalKey<T>::try_with
    22041 (1.6%, 16.0%)    521 (1.3%,  5.2%)  warp::filter::and::_::<impl warp::filter::and::State<T,TE,U>>::project
    20394 (1.5%, 17.5%)    124 (0.3%,  5.5%)  <warp::filter::and_then::State<T,F> as core::future::future::Future>::poll
    19579 (1.4%, 18.9%)     38 (0.1%,  5.6%)  <&mut serde_json::de::Deserializer<R> as serde::de::Deserializer>::deserialize_struct
    18563 (1.4%, 20.3%)    524 (1.3%,  6.9%)  <warp::filter::and::And<T,U> as warp::filter::FilterBase>::filter
    15736 (1.2%, 21.5%)   2248 (5.5%, 12.4%)  core::pin::Pin<P>::new_unchecked
    14916 (1.1%, 22.6%)     44 (0.1%, 12.5%)  alloc::raw_vec::RawVec<T,A>::grow_amortized

After

cargo llvm-lines --lib | head -50
   Compiling warp v0.3.3 (/Users/markus.westerlind/Code/warp)
   Compiling a-server v0.1.0 (/Users/markus.westerlind/Code/a/a-server)
^Pl^[    Finished dev [unoptimized + debuginfo] target(s) in 1m 17s
warning: the following packages contain code that will be rejected by a future version of Rust: buf_redux v0.8.4, multipart v0.18.0
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 1`
  Lines                  Copies               Function name
  -----                  ------               -------------
  1270314                37881                (TOTAL)
    68852 (5.4%,  5.4%)    336 (0.9%,  0.9%)  <warp::filter::and::State<T,TE,U> as core::future::future::Future>::poll
    39285 (3.1%,  8.5%)    135 (0.4%,  1.2%)  <warp::filters::path::Exact<P> as warp::filter::FilterBase>::filter::{{closure}}
    24581 (1.9%, 10.4%)    523 (1.4%,  2.6%)  core::pin::Pin<P>::set
    21120 (1.7%, 12.1%)    192 (0.5%,  3.1%)  std::thread::local::LocalKey<T>::try_with
    20390 (1.6%, 13.7%)    124 (0.3%,  3.5%)  <warp::filter::and_then::State<T,F> as core::future::future::Future>::poll
    19579 (1.5%, 15.3%)     38 (0.1%,  3.6%)  <&mut serde_json::de::Deserializer<R> as serde::de::Deserializer>::deserialize_struct
    16616 (1.3%, 16.6%)    425 (1.1%,  4.7%)  <warp::filter::and::And<T,U> as warp::filter::FilterBase>::filter
    14916 (1.2%, 17.7%)     44 (0.1%,  4.8%)  alloc::raw_vec::RawVec<T,A>::grow_amortized
    14828 (1.2%, 18.9%)    125 (0.3%,  5.1%)  alloc::alloc::box_free

Marwes avatar Mar 30 '23 12:03 Marwes