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

Improve padding performance #20

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kiyonlin
Copy link
Contributor

@kiyonlin kiyonlin commented Oct 21, 2020

Hey @muesli It's me again.

In this PR, I improve padding performance by reusing memory and decreasing memory allocation.

And I don't use Flush in String because we don't need cache to hold the result.

But yeah, MORE code again 😄

OLD:

BenchmarkPaddingString-8   	 3350841	       353 ns/op	     431 B/op	      11 allocs/op
BenchmarkPaddingString-8   	 3336720	       359 ns/op	     432 B/op	      11 allocs/op
BenchmarkPaddingString-8   	 3370746	       354 ns/op	     430 B/op	      11 allocs/op
BenchmarkPaddingString-8   	 3352352	       352 ns/op	     429 B/op	      11 allocs/op
BenchmarkPaddingString-8   	 3432604	       359 ns/op	     431 B/op	      11 allocs/op
BenchmarkPaddingString-8   	 3318981	       354 ns/op	     431 B/op	      11 allocs/op
BenchmarkPaddingString-8   	 3377637	       356 ns/op	     432 B/op	      12 allocs/op
BenchmarkPaddingString-8   	 3282550	       360 ns/op	     432 B/op	      11 allocs/op
BenchmarkPaddingString-8   	 3302713	       367 ns/op	     431 B/op	      11 allocs/op
BenchmarkPaddingString-8   	 3075331	       389 ns/op	     429 B/op	      11 allocs/op

NEW:

BenchmarkPaddingString-8   	 8087502	       147 ns/op	       4 B/op	       0 allocs/op
BenchmarkPaddingString-8   	 8049085	       145 ns/op	       4 B/op	       0 allocs/op
BenchmarkPaddingString-8   	 8362081	       143 ns/op	       4 B/op	       0 allocs/op
BenchmarkPaddingString-8   	 7298126	       147 ns/op	       4 B/op	       1 allocs/op
BenchmarkPaddingString-8   	 7480929	       151 ns/op	       4 B/op	       0 allocs/op
BenchmarkPaddingString-8   	 7328258	       146 ns/op	       4 B/op	       0 allocs/op
BenchmarkPaddingString-8   	 7433443	       156 ns/op	       4 B/op	       0 allocs/op
BenchmarkPaddingString-8   	 7692807	       155 ns/op	       4 B/op	       0 allocs/op
BenchmarkPaddingString-8   	 7617302	       162 ns/op	       4 B/op	       1 allocs/op
BenchmarkPaddingString-8   	 7181878	       162 ns/op	       4 B/op	       0 allocs/op

benchstat:

name             old time/op    new time/op    delta
PaddingString-8     357ns ± 3%     151ns ± 7%   -57.60%  (p=0.000 n=9+10)

name             old alloc/op   new alloc/op   delta
PaddingString-8      431B ± 0%        4B ± 0%   -99.07%  (p=0.000 n=10+10)

name             old allocs/op  new allocs/op  delta
PaddingString-8      11.0 ± 0%       0.0       -100.00%  (p=0.000 n=9+8)

@kiyonlin
Copy link
Contributor Author

kiyonlin commented Nov 3, 2020

Gentle ping @muesli
When you have time, please review this PR and let me know your opinion 😄

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

1 participant