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

String concat allocation reduction #177

Merged
merged 2 commits into from
Jun 28, 2017
Merged

String concat allocation reduction #177

merged 2 commits into from
Jun 28, 2017

Conversation

jmank88
Copy link
Contributor

@jmank88 jmank88 commented Jun 27, 2017

This proposed change offers a ~19% reduction in TreeToTomlString allocations by tweaking some string concatenation and writes, and also fixes some exposed problems with the test Writer, failingWriter.

=== master (/tmp/go-toml-benchmark-cAnF2E/ref)
BenchmarkParseToml-4                 	    1000	   1244751 ns/op	  429140 B/op	   14057 allocs/op
BenchmarkUnmarshalToml-4             	    1000	   1441443 ns/op	  450912 B/op	   15072 allocs/op
BenchmarkUnmarshalBurntSushiToml-4   	    3000	    420625 ns/op	   83486 B/op	    1774 allocs/op
BenchmarkUnmarshalJson-4             	   20000	     66958 ns/op	    3824 B/op	     107 allocs/op
BenchmarkUnmarshalYaml-4             	    5000	    207442 ns/op	   45160 B/op	    1064 allocs/op
BenchmarkLexer-4                     	   10000	    179860 ns/op	   50832 B/op	    1839 allocs/op
BenchmarkTreeToTomlString-4          	  200000	      8640 ns/op	    4960 B/op	      78 allocs/op
PASS
ok  	_/tmp/go-toml-benchmark-cAnF2E/ref	11.966s

=== local
BenchmarkParseToml-4                 	    1000	   1254238 ns/op	  429131 B/op	   14057 allocs/op
BenchmarkUnmarshalToml-4             	    1000	   1403903 ns/op	  450921 B/op	   15072 allocs/op
BenchmarkUnmarshalBurntSushiToml-4   	    3000	    426985 ns/op	   83487 B/op	    1774 allocs/op
BenchmarkUnmarshalJson-4             	   20000	     67378 ns/op	    3824 B/op	     107 allocs/op
BenchmarkUnmarshalYaml-4             	    5000	    208335 ns/op	   45160 B/op	    1064 allocs/op
BenchmarkLexer-4                     	   10000	    176975 ns/op	   50832 B/op	    1839 allocs/op
BenchmarkTreeToTomlString-4          	  200000	      8852 ns/op	    3984 B/op	      63 allocs/op
PASS
ok  	github.com/pelletier/go-toml	12.898s

=== diff
name                       old time/op    new time/op    delta
ParseToml-4                  1.24ms ± 0%    1.25ms ± 0%   +0.76%
UnmarshalToml-4              1.44ms ± 0%    1.40ms ± 0%   -2.60%
UnmarshalBurntSushiToml-4     421µs ± 0%     427µs ± 0%   +1.51%
UnmarshalJson-4              67.0µs ± 0%    67.4µs ± 0%   +0.63%
UnmarshalYaml-4               207µs ± 0%     208µs ± 0%   +0.43%
Lexer-4                       180µs ± 0%     177µs ± 0%   -1.60%
TreeToTomlString-4           8.64µs ± 0%    8.85µs ± 0%   +2.45%

name                       old alloc/op   new alloc/op   delta
ParseToml-4                   429kB ± 0%     429kB ± 0%   -0.00%
UnmarshalToml-4               451kB ± 0%     451kB ± 0%   +0.00%
UnmarshalBurntSushiToml-4    83.5kB ± 0%    83.5kB ± 0%   +0.00%
UnmarshalJson-4              3.82kB ± 0%    3.82kB ± 0%    0.00%
UnmarshalYaml-4              45.2kB ± 0%    45.2kB ± 0%    0.00%
Lexer-4                      50.8kB ± 0%    50.8kB ± 0%    0.00%
TreeToTomlString-4           4.96kB ± 0%    3.98kB ± 0%  -19.68%

name                       old allocs/op  new allocs/op  delta
ParseToml-4                   14.1k ± 0%     14.1k ± 0%    0.00%
UnmarshalToml-4               15.1k ± 0%     15.1k ± 0%    0.00%
UnmarshalBurntSushiToml-4     1.77k ± 0%     1.77k ± 0%    0.00%
UnmarshalJson-4                 107 ± 0%       107 ± 0%    0.00%
UnmarshalYaml-4               1.06k ± 0%     1.06k ± 0%    0.00%
Lexer-4                       1.84k ± 0%     1.84k ± 0%    0.00%
TreeToTomlString-4             78.0 ± 0%      63.0 ± 0%  -19.23%

Related #167

@pelletier
Copy link
Owner

Good catch. Thank you for this patch!

@pelletier pelletier merged commit ef23ce9 into pelletier:master Jun 28, 2017
@jmank88 jmank88 deleted the string_alloc branch June 28, 2017 02:27
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