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: Reduce downloader memory allocations & GC runs #482

Closed
vvelikodny opened this issue Feb 2, 2020 · 2 comments
Closed

Comments

@vvelikodny
Copy link
Contributor

vvelikodny commented Feb 2, 2020

Version of AWS SDK for Go?

v0.19.0

Version of Go (go version)?

go version go1.13.4 darwin/amd64

What issue did you see?

in case the user of the library doesn't create a buffer with a particular size and pass it to the S3 downloader, it downloads file part-by-part from 1st byte up to file length. Thus WriteAtBuffer increase buffer size based on the current max downloaded part, but we know file size at the start after this code:

https://github.com/aws/aws-sdk-go-v2/blob/master/service/s3/s3manager/download.go#L314

d.getChunk()

so I propose download file backward => from the end of the file, so we will allocate the right memory at the start as the last part of the file will be downloaded at the beginning of the process and then buffer never be reallocated. It gives good optimization and GC doesn't work during the whole process, at the beginning only.

Before pull request:
image

After pull request:
image

trace_2_bad_gc.txt
trace_2_good_gc.txt

Steps to reproduce

My sample code:

package main

import (
	"context"
	"fmt"

	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/aws/external"
	"github.com/aws/aws-sdk-go-v2/service/s3"
	"github.com/aws/aws-sdk-go-v2/service/s3/s3manager"
)

func HandleDemoRecord(bucket, region, item string) error {
	fmt.Printf("Bucket = %s, Key = %s\n", bucket, item)

	config, err := external.LoadDefaultAWSConfig()
	if err != nil {
		return err
	}
	config.Region = region

	headObject, err := getHeadObject(bucket, item, config)
	if err != nil {
		return err
	}

	fmt.Printf("obj: %v\nbytes:%+v\n", headObject, *headObject.ContentLength)

	downloader := s3manager.NewDownloader(config, func(d *s3manager.Downloader) {
		d.Concurrency = 5
	})

	var buf []byte
	// allocate before
	//buf := make([]byte, int(*headObject.ContentLength))

	fmt.Println("Starting download of file")
	buffer := aws.NewWriteAtBuffer(buf)
	numBytesDownloaded, err := downloader.Download(buffer, &s3.GetObjectInput{
		Bucket: aws.String(bucket),
		Key:    aws.String(item),
	})

	if err != nil {
		return err
	}

	fmt.Println("Downloaded ", numBytesDownloaded, " bytes")

	return nil
}

func getHeadObject(bucket string, key string, config aws.Config) (*s3.HeadObjectResponse, error) {
	client := s3.New(config)

	input := &s3.HeadObjectInput{
		Bucket: aws.String(bucket),
		Key:    aws.String(key),
	}

	request := client.HeadObjectRequest(input)

	headObjectResponse, err := request.Send(context.TODO())
	if err != nil {
		return nil, err
	}

	fmt.Printf("Downloaded: %v\n", request)

	return headObjectResponse, nil
}

func main() {
	err := HandleDemoRecord("vvelikodny-s3-upload", "ca-central-1", "mysql-workbench-community-8.0.18-macos-x86_64.dmg")
	if err != nil {
		panic(err)
	}
}

here I investigated also memory issues with huge allocations on start, but think it should be in the next issue/pull request.

@vvelikodny vvelikodny changed the title service/s3/s3manager: Fix downloader memory allocation/GC issue service/s3/s3manager: downloader memory allocation/GC issue Feb 2, 2020
@vvelikodny vvelikodny changed the title service/s3/s3manager: downloader memory allocation/GC issue service/s3/s3manager: Reduce memory allocations & GC runs Feb 2, 2020
@vvelikodny vvelikodny changed the title service/s3/s3manager: Reduce memory allocations & GC runs service/s3/s3manager: Reduce downloader memory allocations & GC runs Feb 2, 2020
@github-actions
Copy link

github-actions bot commented Feb 3, 2021

We have noticed this issue has not received attention in 1 year. We will close this issue for now. If you think this is in error, please feel free to comment and reopen the issue.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Feb 3, 2021
@github-actions github-actions bot closed this as completed Feb 8, 2021
@vvelikodny
Copy link
Contributor Author

vvelikodny commented Mar 13, 2021

issue still presented in new SDK #483 (comment)

vvelikodny added a commit to vvelikodny/aws-sdk-go-v2 that referenced this issue Mar 13, 2021
Improve Downloader.Download(..) documentation according aws#482 and aws#483
vvelikodny added a commit to vvelikodny/aws-sdk-go-v2 that referenced this issue Mar 13, 2021
Improve Downloader.Download(..) documentation according aws#482 and aws#483
jasdel pushed a commit that referenced this issue Mar 18, 2021
Improve Downloader.Download(..) documentation according #482 and #483
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant