New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reduce the amount of llvm IR generated #1903
base: master
Are you sure you want to change the base?
Conversation
Reduces the llvm-lines for s3 to 400k from 460k (-13%)
Removes another ~6% of llvm-lines from s3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a maintainer -- just hoping this helps the maintainers.
Just had one question about the change from the HttpDispatch
error variant to Unknown
in a couple places.
let mut response = self.client.sign_and_dispatch(request).await?; | ||
if !response.status.is_success() {{ | ||
let response = response.buffer().await.map_err(RusotoError::HttpDispatch)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the rationale for changing the returned variant from HttpDispatch
to Unknown
here? I'm imagine this might break a few clients, and I would think HttpDispatch
is more appropriate -- but feel free to set me straight here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should still be HttpDispatch
though? It just gets wrapped by the From
impl instead?
let mut response = self.client.sign_and_dispatch(request).await?; | ||
if !response.status.is_success() {{ | ||
let response = response.buffer().await.map_err(RusotoError::HttpDispatch)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as for codegen/query.rs -- not sure about the rationale for changing from the HttpDispatch
variant to Unknown
.
Rusoto generates a lot of code for each of its services which is noticable in its long compile time. These few commits follows the same ideas as in #1773 . Commonly generated code are extracted into helper functions that can be reused and wherever possible this are written in a manner to not have any type parameters so the code is only instantiated once.
On the s3 crate this reduces the amount of generated LLVM IR by ~21% (400099 / 503440 = 0.79473025584)
Before
After