Skip to content
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 #1890

Merged
merged 2 commits into from
Jun 29, 2021
Merged

Reduce the amount of llvm IR generated #1890

merged 2 commits into from
Jun 29, 2021

Conversation

Marwes
Copy link
Contributor

@Marwes Marwes commented Jan 11, 2021

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.

Copy link

@phyber phyber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, however, I am not incredibly familiar with the internals of Rusoto yet, especially the generated code.

@benesch benesch mentioned this pull request Jan 27, 2021
5 tasks
@benesch
Copy link
Contributor

benesch commented Jan 28, 2021

Thanks, @Marwes. The idea here seems good, but this is exactly the kind of large refactoring change that makes me nervous to merge given our current maintenance status (see #1902).

The first two commits are definitely LGTM. If you wanted to split those out those can merge immediately. The other commits would require more time to review than I currently have, as they seem to change behavior at first glance (HttpDispatch error getting removed?).

@Marwes
Copy link
Contributor Author

Marwes commented Jan 29, 2021

Done, submitted the full changes as #1903.

There shouldn't be any changes to the public API from my changes. Not sure what HttpDispatch being removed means?

@benesch benesch merged commit 377ee84 into rusoto:master Jun 29, 2021
@Marwes Marwes deleted the size branch July 1, 2021 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants