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

service/s3/s3manager: Add Upload Buffer Provider #2792

Merged
merged 2 commits into from Sep 17, 2019

Conversation

skmcgrail
Copy link
Member

@skmcgrail skmcgrail commented Aug 28, 2019

Purpose

This change is to help address the performance of s3manager.Uploader in particular on Windows platforms. This PR defines a new BufferStrategy configuration option to the Upload manager which will allow users to define whether a []byte buffer should be used when uploading each part. The reasoning for this new strategy definition, is due to the fact that each part wraps the providedio.ReadSeeker to Uploader in an io.SectionReader which is limited to read from a particular offset and number of bytes. On the Windows platform if the underlying io.ReadSeeker is an *os.File this will result in a large number of blocking events as *os.File.ReadAt uses an emulated pread syscall which requires a mutex lock on the file in order to satisfy the read request. Introducing a BufferStrategy, specifically for Windows results in performance increase, especially in cases where the configured concurrency value for part uploads is high. This change will only enable a buffering strategy by default for the Windows platform, and UNIX-like systems will continue to employ an non-buffering strategy.

Benchmark

Windows

Benchmarks

Summary

Benchmark Difference
BenchmarkUpload/1GB_File/5_Concurrency/5MB_PartSize +0.74%
BenchmarkUpload/1GB_File/5_Concurrency/25MB_PartSize -0.57%
BenchmarkUpload/1GB_File/5_Concurrency/100MB_PartSize -2.70%
BenchmarkUpload/1GB_File/100_Concurrency/5MB_PartSize -24.59%
BenchmarkUpload/1GB_File/100_Concurrency/25MB_PartSize -23.83%
BenchmarkUpload/1GB_File/100_Concurrency/100MB_PartSize -13.68%

Data

goos: windows
goarch: amd64
pkg: github.com/aws/aws-sdk-go/awstesting/integration/performance/s3UploadManager
BenchmarkUpload/1GB_File/5_Concurrency/5MB_PartSize/Unbuffered-32                    100        7074644513 ns/op
BenchmarkUpload/1GB_File/5_Concurrency/5MB_PartSize/Buffered-32                      100        7127001770 ns/op
BenchmarkUpload/1GB_File/5_Concurrency/25MB_PartSize/Unbuffered-32                   100        4452512364 ns/op
BenchmarkUpload/1GB_File/5_Concurrency/25MB_PartSize/Buffered-32                     100        4427085065 ns/op
BenchmarkUpload/1GB_File/5_Concurrency/100MB_PartSize/Unbuffered-32                  100        4027619587 ns/op
BenchmarkUpload/1GB_File/5_Concurrency/100MB_PartSize/Buffered-32                    100        3918910259 ns/op
BenchmarkUpload/1GB_File/100_Concurrency/5MB_PartSize/Unbuffered-32                  100        2503051839 ns/op
BenchmarkUpload/1GB_File/100_Concurrency/5MB_PartSize/Buffered-32                    100        1887655163 ns/op
BenchmarkUpload/1GB_File/100_Concurrency/25MB_PartSize/Unbuffered-32                 100        3160688686 ns/op
BenchmarkUpload/1GB_File/100_Concurrency/25MB_PartSize/Buffered-32                   100        2407384797 ns/op
BenchmarkUpload/1GB_File/100_Concurrency/100MB_PartSize/Unbuffered-32                100        3101042968 ns/op
BenchmarkUpload/1GB_File/100_Concurrency/100MB_PartSize/Buffered-32                  100        2676760109 ns/op
PASS

Linux

Benchmarks

Summary

Benchmark Difference
BenchmarkUpload/1GB_File/5_Concurrency/5MB_PartSize -0.17%
BenchmarkUpload/1GB_File/5_Concurrency/25MB_PartSize -1.90%
BenchmarkUpload/1GB_File/5_Concurrency/100MB_PartSize +1.64%
BenchmarkUpload/1GB_File/100_Concurrency/5MB_PartSize +6.48%
BenchmarkUpload/1GB_File/100_Concurrency/25MB_PartSize +0.67%
BenchmarkUpload/1GB_File/100_Concurrency/100MB_PartSize -7.72%

Data

goos: linux
goarch: amd64
pkg: github.com/aws/aws-sdk-go/awstesting/integration/performance/s3UploadManager
BenchmarkUpload/1GB_File/5_Concurrency/5MB_PartSize/Unbuffered-32                    100        6857056614 ns/op
BenchmarkUpload/1GB_File/5_Concurrency/5MB_PartSize/Buffered-32                      100        6845176279 ns/op
BenchmarkUpload/1GB_File/5_Concurrency/25MB_PartSize/Unbuffered-32                   100        4382487123 ns/op
BenchmarkUpload/1GB_File/5_Concurrency/25MB_PartSize/Buffered-32                     100        4299312985 ns/op
BenchmarkUpload/1GB_File/5_Concurrency/100MB_PartSize/Unbuffered-32                  100        3864865560 ns/op
BenchmarkUpload/1GB_File/5_Concurrency/100MB_PartSize/Buffered-32                    100        3928085242 ns/op
BenchmarkUpload/1GB_File/100_Concurrency/5MB_PartSize/Unbuffered-32                  100        1651138406 ns/op
BenchmarkUpload/1GB_File/100_Concurrency/5MB_PartSize/Buffered-32                    100        1758210520 ns/op
BenchmarkUpload/1GB_File/100_Concurrency/25MB_PartSize/Unbuffered-32                 100        2160376327 ns/op
BenchmarkUpload/1GB_File/100_Concurrency/25MB_PartSize/Buffered-32                   100        2174799197 ns/op
BenchmarkUpload/1GB_File/100_Concurrency/100MB_PartSize/Unbuffered-32                100        2739669395 ns/op
BenchmarkUpload/1GB_File/100_Concurrency/100MB_PartSize/Buffered-32                  100        2528230455 ns/op

@skmcgrail skmcgrail requested a review from jasdel August 28, 2019 18:11
@skmcgrail skmcgrail added the needs-review This issue or pull request needs review from a core team member. label Aug 28, 2019
@skmcgrail skmcgrail force-pushed the s3manager/buffering branch 6 times, most recently from afa1a91 to cd98853 Compare August 30, 2019 20:47
Copy link
Contributor

@jasdel jasdel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the cases were static numbers are use, i think it may be easier to ready if inline multiplication is used for the varing sizes, e.g. 1024*1024*5 for 5MB

service/s3/s3manager/buffered_read_seeker.go Outdated Show resolved Hide resolved
service/s3/s3manager/buffered_read_seeker_test.go Outdated Show resolved Hide resolved
service/s3/s3manager/upload.go Outdated Show resolved Hide resolved
service/s3/s3manager/upload.go Show resolved Hide resolved
service/s3/s3manager/upload.go Outdated Show resolved Hide resolved
service/s3/s3manager/upload.go Outdated Show resolved Hide resolved
service/s3/s3manager/buffered_read_seeker.go Outdated Show resolved Hide resolved
service/s3/s3manager/buffered_read_seeker.go Outdated Show resolved Hide resolved
@skmcgrail skmcgrail force-pushed the s3manager/buffering branch 2 times, most recently from 49f871a to edc26b1 Compare September 16, 2019 23:49
@skmcgrail skmcgrail changed the title service/s3/s3manager: Add BufferStrategy for Defining Buffering of Upload Parts service/s3/s3manager: Add Upload Buffer Provider Sep 16, 2019
@skmcgrail skmcgrail removed the needs-review This issue or pull request needs review from a core team member. label Sep 17, 2019
@skmcgrail skmcgrail merged commit 487e411 into aws:master Sep 17, 2019
@skmcgrail skmcgrail deleted the s3manager/buffering branch September 17, 2019 16:09
@aws-sdk-go-automation aws-sdk-go-automation mentioned this pull request Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants