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

Avoid creating unnecessary intermediate strings #3415

Closed
1 of 2 tasks
paulomorgado opened this issue Aug 2, 2024 · 5 comments
Closed
1 of 2 tasks

Avoid creating unnecessary intermediate strings #3415

paulomorgado opened this issue Aug 2, 2024 · 5 comments
Labels
feature-request A feature should be added or improved. module/sdk-core p2 This is a standard priority issue queued

Comments

@paulomorgado
Copy link

paulomorgado commented Aug 2, 2024

Describe the feature

I found in the code base code like this:

            using (StringWriter stringWriter = new StringWriter(CultureInfo.InvariantCulture))
            {
                // ...
                string snippet = stringWriter.ToString();
                request.Content = System.Text.Encoding.UTF8.GetBytes(snippet);
            }

StringWriter uses an internal StringBuilder that will be used to produce a string just to get the utf-8 bytes.

Use Case

Creating objects that do not need to be created, not only uses memory and CPU to create them, but also causes GC work.

Proposed Solution

Consider using a StreamWriter instead:

using (var ms = new MemoryStream())
{
    using (var writer = new new StreamWriter(ms))
    {
       // ...
    }
    request.Content = ms.ToArray();
}

Other Information

If IRequest.Content was a ArraySegment<byte>, this:

request.Content = ms.ToArray();

coulde be this:

request.Content = new ArraySegment<byte>(ms.GetBuffer(), 0, (int)ms.Length);

which is one less byte[] allocation.

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

AWS .NET SDK and/or Package version used

AWSSDK.KeyManagementService 3.7.300.52

Targeted .NET Platform

.NET 8

Operating System and version

Windows and Linux

@paulomorgado paulomorgado added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 2, 2024
@bhoradc bhoradc added module/sdk-core p2 This is a standard priority issue needs-review and removed needs-triage This issue or PR still needs to be triaged. labels Aug 2, 2024
@bhoradc
Copy link

bhoradc commented Aug 2, 2024

Hello @paulomorgado,

Thank you for taking the time to share your performance optimization findings! Since you are open to contributing a PR, please feel free to do so. Team will be happy to review it.

Regards,
Chaitanya

@paulomorgado
Copy link
Author

StringWriter

Just under sdk and not counting sdk/test, it's about 9500 files.

Is the code base covered in tests for all that?

@paulomorgado
Copy link
Author

@bhoradc,

I think I figured out what I needed to change: generator\ServiceClientGeneratorLib\Generators\Marshallers\JsonRPCRequestMarshaller.tt

But I couldn't figure out how to generate the clients.

See #3439

@dscpinheiro
Copy link
Contributor

Your PR has been merged and will be included in the next preview release of the SDK.

Thanks again for the contribution!

Copy link

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. module/sdk-core p2 This is a standard priority issue queued
Projects
None yet
Development

No branches or pull requests

3 participants