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

[S3] Fixing PutObject High Memory Usage #899

Merged
merged 1 commit into from Mar 29, 2018

Conversation

@indy-singh
Copy link
Contributor

commented Mar 13, 2018

See #894 for detailed description

Description

Previously, when invoking the constructor of ChunkedUploadWrapperStream it used to create two byte arrays each with a size of 131,072 (128*1024). From a memory aspect this is inefficient as objects over 85,000 bytes go straight into an area of memory known as the Large Object Heap (LOH). This area is never compacted by default (as compaction of the LOH is expensive), but is collected. Over a lifetime of an application it is possible to end up in a situation where the LOH has taken up most of the RAM.

Now, the byte arrays are still created but with a size of 81,920. The reason for this number can be found here: https://referencesource.microsoft.com/#mscorlib/system/io/stream.cs,50. As 81,920 is under the 85,000 threshold the byte arrays in question go onto the Small Object Heap (SOH). This area of memory is collected and compacted by default.

Motivation and Context

To reduce memory consumption of this call:-

public virtual PutObjectResponse PutObject(PutObjectRequest request)

Total SOH LOH
Before 114MB 64MB 50MB
After 98MB 98MB 0.4MB

Testing

See #894 for benchmarks within a test harness for pre/post change.

Screenshots (if appropriate)

See #894 for screenshots

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license
See #894 for detailed description
@indy-singh indy-singh changed the title Lowering DefaultChunkSize [S3] Fixing PutObject High Memory Usage Mar 14, 2018
@indy-singh

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2018

@sstevenkang Is there anything I can do here to get this PR merged?

Thanks,
Indy

@sstevenkang

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2018

Interesting. Let me check with the team regarding the history of choosing 128*1024 as the default size.

@sstevenkang sstevenkang self-assigned this Mar 21, 2018
@indy-singh

This comment has been minimized.

Copy link
Contributor Author

commented Mar 26, 2018

Hi @sstevenkang,

Any news?

Thanks,
Indy

@spati2 spati2 merged commit 008c5ff into aws:master Mar 29, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@normj

This comment has been minimized.

Copy link
Member

commented Mar 29, 2018

Thanks for the pull request. This went out with version 3.3.21.19 of AWSSDK.Core

@indy-singh

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2018

Thanks for accepting this, really appreciate it. FYI GetPreSignedURL offers much better performance within a memory context; #894 (comment)

@indy-singh indy-singh deleted the indy-singh:s3-loh-changes branch Apr 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.