-
Notifications
You must be signed in to change notification settings - Fork 215
Add decorator and setting to set hints.mostly-unused
#4243
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
Conversation
@@ -17,6 +17,14 @@ fun ClientCodegenDecorator.onlyApplyTo(serviceId: String): List<ClientCodegenDec | |||
}, | |||
) | |||
|
|||
/** Only apply this decorator to the given list of service IDs */ |
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 didn't end up needing this after I moved the service list to gradle.properties
, but seemed like it could be a useful utility in the future, so I left it.
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
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.
Excited to get this out the door!
BTW, people have also complained about aws-sdk-ssm
in this issue. Should we consider adding it to the list of services?
Looks like this comment has a pretty good list, if we all feel comfortable with it I can add:
To the existing list |
Added the discussed crates in 550bc3f, will do some quick bench marks on them to make sure it is beneficial. The failing action (for generating diffs) has the error:
Which makes me think it might be related to #4039 that was merged earlier today. Checking that PR it seems like the diff isn't generated for the CI run on forks, so CI there wouldn't have caught it. |
I would suggest also adding Without the hint, a crate depending on |
...ient/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustSettings.kt
Outdated
Show resolved
Hide resolved
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
aws.services= | ||
aws.services.hints.mostlyUnused=\ |
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.
Probably want to remove this.
## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here --> ## Description <!--- Describe your changes in detail --> ## Testing <!--- Please describe in detail how you tested your changes --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
Motivation and Context
Continuing the work started in #4208, the new feature we are utilizing is explained in https://blog.rust-lang.org/inside-rust/2025/07/15/call-for-testing-hint-mostly-unused
Description
Add a new
ManifestHintsDecorator
that allows adding a[hints]
section to a generatedCargo.toml
. This also introduces a new optional entry insmithy-build.json
,hintsMostlyUnusedList
that allows indicating which crates this should be enabled for. Currently this is only enabled foraws-sdk-s3
,aws-sdk-dynamodb
, andaws-sdk-ec2
.Testing
I tested both debug and release builds of the crates, with and without the hint on an M1 Macbook Pro. You can see the full data below, but the general outcome is that release builds are ~50% faster with the hint and debug builds are ~40% faster. A release build of the
aws-sdk-ec2
crate went from124.1s
without the hint to59.1s
with it.Expand To See Test Data
All tests run after deleting the `target/` dir
Debug No Hint
cargo build --timings
Debug Hint
cargo +nightly -Zprofile-hint-mostly-unused build --timings
Release No Hint
cargo build --timings --release
Release Hint
cargo +nightly -Zprofile-hint-mostly-unused build --timings --release
Checklist
.changelog
directory, specifying "client," "server," or both in theapplies_to
key..changelog
directory, specifying "aws-sdk-rust" in theapplies_to
key.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.