The Wayback Machine - https://web.archive.org/web/20211006154112/https://github.com/segmentio/kafka-go/pull/318
Skip to content

/ kafka-go Public

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

Jump to bottom

kafka.writeBuffer #318

Merged
merged 2 commits into from Jul 22, 2019
Merged

kafka.writeBuffer #318

merged 2 commits into from Jul 22, 2019

Conversation

@achille-roussel
Copy link
Contributor

@achille-roussel achille-roussel commented Jul 22, 2019 •
edited

This PR refactors the write paths to remove the intermediary buffer that we used to compute checksums of record batches. These temporary buffers would end up being really large as they would have to hold the entire batch, depending on the buffering strategy this could end up being multiple MB of memory allocated on each write to kafka.

Memory footprint on writes should be greatly reduced by this change:

benchmark               old ns/op     new ns/op     delta
BenchmarkConn/Write     1026737       1004170       -2.20%

benchmark               old MB/s     new MB/s     speedup
BenchmarkConn/Write     9.74         9.96         1.02x

benchmark               old allocs     new allocs     delta
BenchmarkConn/Write     10             7              -30.00%

benchmark               old bytes     new bytes     delta
BenchmarkConn/Write     14738         322           -97.82%

Varint encoding is also much faster now that we work on a local buffer:

benchmark                old ns/op     new ns/op     delta
BenchmarkWriteVarInt     34.5          9.70          -71.88%
BenchmarkReadVarInt      50.1          47.9          -4.39%

benchmark                old allocs     new allocs     delta
BenchmarkWriteVarInt     0              0              +0.00%
BenchmarkReadVarInt      0              0              +0.00%

benchmark                old bytes     new bytes     delta
BenchmarkWriteVarInt     0             0             +0.00%
BenchmarkReadVarInt      0             0             +0.00%
@achille-roussel
add kafka.writeBuffer
734805d
@achille-roussel
fix crc32 computation
feb6f42
@achille-roussel achille-roussel requested review from stevevls, Pryz and rjenkins and removed request for stevevls Jul 22, 2019
rjenkins
rjenkins approved these changes Jul 22, 2019
View changes
Copy link
Member

@rjenkins rjenkins left a comment

lgtm!

stevevls
stevevls approved these changes Jul 22, 2019
View changes
Copy link
Contributor

@stevevls stevevls left a comment

Nice find.

@achille-roussel
Copy link
Contributor Author

@achille-roussel achille-roussel commented Jul 22, 2019

Thanks for the reviews!

@achille-roussel achille-roussel merged commit ad7818b into master Jul 22, 2019
4 checks passed
@achille-roussel achille-roussel deleted the write-buffer branch Jul 22, 2019
@achille-roussel achille-roussel mentioned this pull request Jul 23, 2019
Merged
@achille-roussel achille-roussel mentioned this pull request Jul 24, 2019
Closed
@sagarkrkv sagarkrkv mentioned this pull request Aug 1, 2019
Closed
@sagarkrkv sagarkrkv mentioned this pull request Sep 9, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevevls stevevls

@rjenkins rjenkins

@Pryz Pryz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@achille-roussel @stevevls @rjenkins