Eliminate []byte(string) allocation under thrift transport #729

jcorbin · 2019-03-29T21:38:46Z

No description provided.

prashantv

Looks like we have code coverage for both paths (writer implements WriteString or does not).

prashantv · 2019-03-29T23:19:23Z

thrift/transport.go

+	b := append(t.strBuf[:0], s...)
+	t.strBuf = b[:0]
+	return t.Writer.Write(b)
+


nit: delete blank line

codecov · 2019-04-01T20:57:42Z

Codecov Report

Merging #729 into dev will decrease coverage by 0.22%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev     #729      +/-   ##
==========================================
- Coverage   87.77%   87.55%   -0.23%     
==========================================
  Files          40       40              
  Lines        4075     4075              
==========================================
- Hits         3577     3568       -9     
- Misses        377      388      +11     
+ Partials      121      119       -2

Impacted Files	Coverage Δ
connection.go	`84.59% <0%> (-2.21%)`	⬇️
mex.go	`72.09% <0%> (-1.87%)`	⬇️
preinit_connection.go	`92.7% <0%> (-1.46%)`	⬇️
peer.go	`93.81% <0%> (+0.36%)`	⬆️
relay.go	`82.09% <0%> (+0.61%)`	⬆️
outbound.go	`87.93% <0%> (+1.72%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a255dc1...cd8e8c7. Read the comment docs.

jcorbin requested a review from prashantv March 29, 2019 21:38

prashantv approved these changes Mar 29, 2019

View reviewed changes

thrift/transport.go Outdated

b := append(t.strBuf[:0], s...)

t.strBuf = b[:0]

return t.Writer.Write(b)

Copy link

Contributor

prashantv Mar 29, 2019

Choose a reason for hiding this comment

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

nit: delete blank line

jcorbin force-pushed the thrift_write_string_alloc branch from 223d473 to 02060b8 Compare April 1, 2019 20:57

Eliminate []byte(string) allocation under thrift transport

cd8e8c7

jcorbin force-pushed the thrift_write_string_alloc branch from 02060b8 to cd8e8c7 Compare April 1, 2019 21:38

jcorbin merged commit 016d215 into dev Apr 1, 2019

jcorbin deleted the thrift_write_string_alloc branch April 1, 2019 21:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eliminate []byte(string) allocation under thrift transport #729

Eliminate []byte(string) allocation under thrift transport #729

jcorbin commented Mar 29, 2019

prashantv left a comment

prashantv Mar 29, 2019

codecov bot commented Apr 1, 2019 •

edited

Loading

Eliminate []byte(string) allocation under thrift transport #729

Eliminate []byte(string) allocation under thrift transport #729

Conversation

jcorbin commented Mar 29, 2019

prashantv left a comment

Choose a reason for hiding this comment

prashantv Mar 29, 2019

Choose a reason for hiding this comment

codecov bot commented Apr 1, 2019 • edited Loading

Codecov Report

codecov bot commented Apr 1, 2019 •

edited

Loading