aws-sdk-rust
aws-sdk-rust copied to clipboard
[request]: Accept borrowed data types as inputs
Community Note
- Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
- Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
- If you are interested in working on this issue, please leave a comment
Tell us about your request Would it be possible to accept borrowed inputs for builders? That is, instead of always making an allocation when accepting a string, make it possible for the caller to provide a borrowed type.
The allocations may seem negligible, but as a foundational library, aws-sdk-rust might as well design its APIs to be as low-cost as possible, while still being ergonomic.
Concrete example
https://docs.rs/aws-sdk-dynamodb/0.0.25-alpha/aws_sdk_dynamodb/client/fluent_builders/struct.GetItem.html#method.table_name
Every time GetItem::table_name is called, a String allocation needs to be made. If I have a &'static str it feels wasteful to incur an allocation for a String.
Tell us about the problem you're trying to solve. What are you trying to do, and why is it hard? I would like to not pay the price of allocations when that is not needed.
AFAICT there is no inherent need for the builder to own the input data. The builder only needs read-access for constructing a request to the AWS API.
Solution ideas
I'll use &str and String as examples, but also applies for other data types.
Made a small Playground link for exploring different ideas: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=e61a33a1aa8b631aa6b328ea93fd6b4b
For all the new designs, the builder struct will need a lifetime parameter, which is a bit less ergonomic versus the status-quo.
set_stringis the current API. Storage in builder is 24 bytes.set_cowuses aCow<'a, str>type as storage, which is 32 bytes. The caller can provide both owned types and borrowed types.set_borroweduses a&strtype as storage, which is 16 bytes. I don't know how to lose the&*in the call using aString.set_strarguable the simplest implementation, but also least generic. Only allows passing borrowed data. It does clearly tell the caller that the library only needs read-access.
Are you currently working around this issue? The current workaround is incurring the allocation cost.
thanks for the request! This is something we've been considering, however, we haven't tackled it yet for two reasons:
- The impact is quite marginal considering that you're going to eventually dispatch an HTTP request over the network
- It can make the builders much more cumbersome to use, especially in closures since the builders now need to carry around a lifetime.
So although this is a change we're considering, it requires very careful study of the tradeoffs. If you have performance metrics demonstrating that the extra allocations current incurred are a problem, that would also be a great data point to drive prioritizations.
For other community members
If this is important to you, please upvote this issue!
I'd add my support here in something at least less allocation heavy than always a String, at least for the DynamoDB SDK. I would be happy to have the API accept impl Into<Cow<'static, str>> if there was a desire to avoid a lifetime on the fluent builders, but for heavy usage, the number of repeated tiny allocations for attribute names or expressions that are just &'static str raises my antennae. You're probably right that in the grand scheme of things, with a network call on the other side, the cost isn't huge, though.