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: Use interfaces assertions instead of ValuesAtPath for S3 field lookups. #1401

Merged
merged 3 commits into from
Jul 18, 2017

Conversation

jasdel
Copy link
Contributor

@jasdel jasdel commented Jul 13, 2017

Improves the performance across the board for all S3 API calls by removing the usage of ValuesAtPath being used for every S3 API call.

With a simple S3 HeadObject call the following benchmarks improved the performance of the API call significantly.

benchmark                   old ns/op     new ns/op     delta
BenchmarkS3HeadObject-4     190419        169992        -10.73%

benchmark                   old allocs     new allocs     delta
BenchmarkS3HeadObject-4     511            444            -13.11%

benchmark                   old bytes     new bytes     delta
BenchmarkS3HeadObject-4     68694         61968         -9.79%
package main

import (
	"net/http"
	"net/http/httptest"
	"testing"

	"github.com/aws/aws-sdk-go/aws"
	"github.com/aws/aws-sdk-go/aws/credentials"
	"github.com/aws/aws-sdk-go/aws/endpoints"
	"github.com/aws/aws-sdk-go/aws/session"
	"github.com/aws/aws-sdk-go/service/s3"
)

func BenchmarkS3HeadObject(b *testing.B) {
	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.WriteHeader(http.StatusOK)
	}))

	sess := session.Must(session.NewSession(&aws.Config{
		Credentials:      credentials.NewStaticCredentials("Key", "Secret", "Token"),
		Endpoint:         aws.String(server.URL),
		S3ForcePathStyle: aws.Bool(true),
		DisableSSL:       aws.Bool(true),
		Region:           aws.String(endpoints.UsWest2RegionID),
	}))
	svc := s3.New(sess)
	b.ResetTimer()

	for i := 0; i < b.N; i++ {
		req, _ := svc.HeadObjectRequest(&s3.HeadObjectInput{
			Bucket:    aws.String("somebucketname"),
			Key:       aws.String("keyname"),
			VersionId: aws.String("someVersion"),
			IfMatch:   aws.String("IfMatch"),
		})
		if err := req.Send(); err != nil {
			b.Fatal(err)
		}
	}
}

@jasdel jasdel self-assigned this Jul 13, 2017
@jasdel jasdel requested a review from xibz July 13, 2017 22:29
}
if iface, ok := params.(bucketGetter); ok {
b := iface.getBucket()
return b, len(b) > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

What would this function look like if an int64 was returned instead of a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

discussed.

@jasdel jasdel merged commit 4ac69ad into aws:master Jul 18, 2017
@jasdel jasdel deleted the feature/NoS3JSONPath branch July 18, 2017 19:49
jasdel added a commit that referenced this pull request Jul 18, 2017
@awstools awstools mentioned this pull request Jul 19, 2017
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.

2 participants