Skip to content

Improve memory allocation behavior by replacing sync.Pool with custom pool implementation #3183

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

Merged
merged 14 commits into from
Mar 9, 2020

Conversation

skmcgrail
Copy link
Member

@skmcgrail skmcgrail commented Mar 4, 2020

Pool Benchmarks

name                  time/op
Pools/sync.Pool/0-72  23.3ms ± 6%
Pools/sync.Pool/1-72  44.1ms ± 3%
Pools/sync.Pool/2-72  51.6ms ±20%
Pools/sync.Pool/3-72   128ms ± 1%
Pools/custom/0-72     23.0ms ± 5%
Pools/custom/1-72     44.5ms ± 1%
Pools/custom/2-72     47.1ms ± 2%
Pools/custom/3-72      131ms ± 7%

name                  alloc/op
Pools/sync.Pool/0-72  10.6MB ± 0%
Pools/sync.Pool/1-72  15.9MB ± 0%
Pools/sync.Pool/2-72  26.6MB ± 0%
Pools/sync.Pool/3-72   164MB ± 0%
Pools/custom/0-72     10.6MB ± 0%
Pools/custom/1-72     10.7MB ± 0%
Pools/custom/2-72     16.0MB ± 0%
Pools/custom/3-72     59.0MB ± 0%

name                  allocs/op
Pools/sync.Pool/0-72   1.03k ± 0%
Pools/sync.Pool/1-72   1.29k ± 0%
Pools/sync.Pool/2-72   1.82k ± 0%
Pools/sync.Pool/3-72   13.4k ± 0%
Pools/custom/0-72      1.03k ± 0%
Pools/custom/1-72      1.28k ± 0%
Pools/custom/2-72      1.80k ± 0%
Pools/custom/3-72      13.4k ± 0%

Multi-Part Upload

  • 150 GB File
  • 16 MB PartSize
  • 72 Concurrency

Expected Buffer Allocations: (72+1)*16 = 1168 MB

Before (sync.Pool)

As shown sync.Pool drastically performs over allocations, 2624 MB. This is over 124% more in allocations!

Old Pool

After

We see that we now allocate the exact amount of expected buffers, 1168 MB.

New Pool

Performance Comparison

$ ~/go/bin/benchstat /mnt/ram/results.old /mnt/ram/results.new
name                                                         old time/op    new time/op    delta
Upload/540MB_File/72_Concurrency/5MB_PartSize/Unbuffered-72     1.50s ±27%     1.57s ±31%   +4.52%  (p=0.006 n=84+89)

name                                                         old alloc/op   new alloc/op   delta
Upload/540MB_File/72_Concurrency/5MB_PartSize/Unbuffered-72     595MB ± 0%     407MB ± 0%  -31.57%  (p=0.000 n=96+99)

name                                                         old allocs/op  new allocs/op  delta
Upload/540MB_File/72_Concurrency/5MB_PartSize/Unbuffered-72      151k ± 0%      151k ± 0%   +0.06%  (p=0.009 n=86+88)

@skmcgrail skmcgrail added the pr/work-in-progress This PR is a draft and needs further work. label Mar 4, 2020
@skmcgrail skmcgrail requested a review from jasdel March 4, 2020 18:21
@skmcgrail skmcgrail added pr/needs-review This PR needs a review from a Member. and removed pr/work-in-progress This PR is a draft and needs further work. labels Mar 5, 2020
@skmcgrail skmcgrail force-pushed the s3manager/memory branch 3 times, most recently from 37759dd to 4ea1776 Compare March 5, 2020 23:56
@skmcgrail skmcgrail requested a review from jasdel March 5, 2020 23:57
@skmcgrail skmcgrail requested a review from jasdel March 7, 2020 00:53
@skmcgrail skmcgrail merged commit a837662 into aws:master Mar 9, 2020
@skmcgrail skmcgrail deleted the s3manager/memory branch March 9, 2020 22:54
aws-sdk-go-automation pushed a commit that referenced this pull request Mar 10, 2020
===

### Service Client Updates
* `service/ec2`: Updates service API and documentation
  * Documentation updates for EC2
* `service/iotevents`: Updates service API and documentation
* `service/marketplacecommerceanalytics`: Updates service documentation
  * Change the disbursement data set to look past 31 days instead until the beginning of the month.
* `service/serverlessrepo`: Updates service API and documentation

### SDK Enhancements
* `aws/credentials`: Clarify `token` usage in `NewStaticCredentials` documentation.
  * Related to [#3162](#3162).
* `service/s3/s3manager`: Improve memory allocation behavior by replacing sync.Pool with custom pool implementation ([#3183](#3183))
  * Improves memory allocations that occur when the provided `io.Reader` to upload does not satisfy both the `io.ReaderAt` and `io.ReadSeeker` interfaces.
  * Fixes [#3075](#3075)
aws-sdk-go-automation added a commit that referenced this pull request Mar 10, 2020
Release v1.29.21 (2020-03-10)
===

### Service Client Updates
* `service/ec2`: Updates service API and documentation
  * Documentation updates for EC2
* `service/iotevents`: Updates service API and documentation
* `service/marketplacecommerceanalytics`: Updates service documentation
  * Change the disbursement data set to look past 31 days instead until the beginning of the month.
* `service/serverlessrepo`: Updates service API and documentation

### SDK Enhancements
* `aws/credentials`: Clarify `token` usage in `NewStaticCredentials` documentation.
  * Related to [#3162](#3162).
* `service/s3/s3manager`: Improve memory allocation behavior by replacing sync.Pool with custom pool implementation ([#3183](#3183))
  * Improves memory allocations that occur when the provided `io.Reader` to upload does not satisfy both the `io.ReaderAt` and `io.ReadSeeker` interfaces.
  * Fixes [#3075](#3075)
skotambkar pushed a commit to skotambkar/aws-sdk-go that referenced this pull request May 20, 2021
* Uploader should initialize pool size after determining optimized size. (aws#3030)
* Fix resource leak on failed CreateMultipartUpload calls (aws#3069)
* Fix resource leak on UploadPart failures (aws#3144)
* Improve memory allocations by replacing sync.Pool (aws#3183)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/needs-review This PR needs a review from a Member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

memory usage of s3 upload manager for Go 1.13.5 and 1.13.6
2 participants