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

aws/signer/v4: Optimize stripExcessSpaces #1417

Merged
merged 1 commit into from
Jul 25, 2017

Conversation

stevenh
Copy link
Contributor

@stevenh stevenh commented Jul 25, 2017

Optimised stripExcessSpaces by using a hand rolled trim space avoiding the overhead of strings.TrimSpace and ensuring we only do a single pass on the string for both search and copy when stripping multiple spaces.

This improves the performance on the test machine from 2378 ns/op to 1528 ns/op while keeping allocs the same.

Also:

  • Added a blank and only trialing spaces test cases.

Optimised stripExcessSpaces by using a hand rolled trim space avoiding the overhead of strings.TrimSpace and ensuring we only do a single pass on the string for both search and copy when stripping multiple spaces.

This improves the performance on the test machine from 2378 ns/op to 1528 ns/op while keeping allocs the same.

Also:
* Added a blank and only trialing spaces test cases.
@stevenh
Copy link
Contributor Author

stevenh commented Jul 25, 2017

Travis CI failure looks totally unrelated.

@jasdel
Copy link
Contributor

jasdel commented Jul 25, 2017

Thanks for putting this change together @stevenh. The change looks good. We'll pull this in, and will be included in our next release.

@jasdel jasdel changed the title Optimised stripExcessSpaces performance aws/signer/v4: Optimize stripExcessSpaces Jul 25, 2017
@jasdel jasdel merged commit 66fbc91 into aws:master Jul 25, 2017
jasdel added a commit that referenced this pull request Jul 25, 2017
This was referenced Jul 25, 2017
@stevenh stevenh deleted the optimise-strip-spaces branch August 17, 2017 14:05
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