Skip to content

Conversation

bemasc
Copy link

@bemasc bemasc commented Apr 9, 2020

Currently, shadowsocksReader.Read passes a []byte to
a method that takes an interface{}, triggering an
automatic cast that results in a memory allocation.

This change refactors the reader code to avoid this allocation,
with no change in behavior otherwise.

Currently, shadowsocksReader.Read passes a `[]byte` to
a method that takes an `interface{}`, triggering an
automatic cast that results in a memory allocation.

This change refactors the reader code to avoid this allocation,
with no change in behavior otherwise.
@bemasc bemasc requested review from fortuna and alalamav April 9, 2020 00:10
Copy link

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

Thanks for the improvement. These extra allocations are something I'm starting to dislike about Go... :-/

// waits for incoming data and decrypts it.
// Returns an error only if sr.leftover could not be populated.
func (sr *shadowsocksReader) populateLeftover() error {
if len(sr.leftover) == 0 {
Copy link

Choose a reason for hiding this comment

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

Return early if >0 to remove nesting and improve readability.

Copy link
Author

Choose a reason for hiding this comment

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

Done

// Ensures that sr.leftover is nonempty. If leftover is empty, this method
// waits for incoming data and decrypts it.
// Returns an error only if sr.leftover could not be populated.
func (sr *shadowsocksReader) populateLeftover() error {
Copy link

Choose a reason for hiding this comment

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

Maybe call this ensureLeftover(), since it doesn't always populate it? It will align with your comment ("Ensures....")

Copy link
Author

Choose a reason for hiding this comment

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

Done

@bemasc bemasc merged commit a613e7b into master Apr 13, 2020
@bemasc bemasc deleted the bemasc-cast branch April 13, 2020 23:41
@fortuna fortuna added the memory usage Improvements in memory usage label May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
memory usage Improvements in memory usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants