Skip to content

fix: avoid unnecessary Write in FileBackend#159

Merged
lucaspin merged 2 commits intosemaphoreci:masterfrom
lyoung-confluent:patch-1
Aug 5, 2022
Merged

fix: avoid unnecessary Write in FileBackend#159
lucaspin merged 2 commits intosemaphoreci:masterfrom
lyoung-confluent:patch-1

Conversation

@lyoung-confluent
Copy link
Copy Markdown
Contributor

Was just skimming the code and noticed a minor optimization in FileBackend. Currently it calls (*os.File).Write twice, once for the JSON line then to add a \n. Because this is not a buffered io.Writer (i.e. bufio) this could result in unnecessary write syscalls and resulting disk I/O. Instead you can just append the \n in memory and call Write once.

@lucaspin
Copy link
Copy Markdown
Contributor

lucaspin commented Aug 4, 2022

/sem-approve

2 similar comments
@lucaspin
Copy link
Copy Markdown
Contributor

lucaspin commented Aug 4, 2022

/sem-approve

@radwo
Copy link
Copy Markdown
Member

radwo commented Aug 4, 2022

/sem-approve

@lucaspin
Copy link
Copy Markdown
Contributor

lucaspin commented Aug 4, 2022

@lyoung-confluent nice find, and thanks for the PR. Would you be able to update your branch with the latest master? There was an issue with CI, which led your build to fail. The issue was fixed in #160, so if you pull the latest master into your branch, CI should pass.

@radwo
Copy link
Copy Markdown
Member

radwo commented Aug 5, 2022

/sem-approve

@lucaspin lucaspin self-requested a review August 5, 2022 14:16
@lucaspin lucaspin merged commit 3fa555a into semaphoreci:master Aug 5, 2022
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.

3 participants