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

internal/protocol/json: Reduce unmarshal allocs #376

Merged
merged 1 commit into from
Sep 8, 2015

Conversation

bpot
Copy link
Contributor

@bpot bpot commented Sep 7, 2015

Check PkgPath on StructField which will be empty for exported fields
(http://golang.org/pkg/reflect/#StructField).

We have an application which heavily uses DynamoDB and we are seeing a lot of allocations in jsonutil/unmarshal. Based on a heap dump nearly 25% of the allocations (program wide) are coming from strings.ToLower in unmarshalStruct:

(pprof) peek ToLower
241578716 of 252105072 total (95.82%)
Dropped 547 nodes (cum <= 1260525)
----------------------------------------------------------+-------------
      flat  flat%   sum%        cum   cum%   calls calls% + context          
----------------------------------------------------------+-------------
                                          57705326   100% |   github.com/aws/aws-sdk-go/internal/protocol/json/jsonutil.unmarshalStruct
         0     0%     0%   57836399 22.94%                | strings.ToLower
                                          57836399   100% |   strings.Map
----------------------------------------------------------+-------------

Check PkgPath on StructField which will be empty for exported fields
(http://golang.org/pkg/reflect/#StructField).
@bpot
Copy link
Contributor Author

bpot commented Sep 7, 2015

This is the same technique that the json standard library uses (https://github.com/golang/go/blob/932c1e3dd32f636ab3f25b23d9dcef194a577bca/src/encoding/json/encode.go#L1025) to check if a field is export so it should be correct.

@jasdel
Copy link
Contributor

jasdel commented Sep 8, 2015

Great find @bpot ! Thanks for submitting this PR. Allocations of the protocol marshallers is something we definitely want to improve. There are a series of benchmarks in internal/test/perf/protocol, but only for marshallers. None for unmarshalling have been added yet.

I noticed this issue is present in several of the other marshallers, so similar fixes might be able to be applied to them also. I created #377 to track improving the protocol marshallers in general.

jasdel added a commit that referenced this pull request Sep 8, 2015
internal/protocol/json: Reduce unmarshal allocs
@jasdel jasdel merged commit 59b2dc5 into aws:master Sep 8, 2015
@bpot
Copy link
Contributor Author

bpot commented Sep 8, 2015

Thanks!

jasdel added a commit that referenced this pull request Sep 10, 2015
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