Skip to content

simplify buffer resizing - #860

Merged
roderickvd merged 1 commit into
librespot-org:devfrom
JasonLG1979:simplify-buffer-resizing
Oct 8, 2021
Merged

simplify buffer resizing#860
roderickvd merged 1 commit into
librespot-org:devfrom
JasonLG1979:simplify-buffer-resizing

Conversation

@JasonLG1979

Copy link
Copy Markdown
Contributor

This way is less verbose, much more simple and less brittle.

@JasonLG1979

Copy link
Copy Markdown
Contributor Author

@roderickvd I wasn't happy with the match and trying to be "clever" and doing different things when trying to lower or raise the capacity seemed brittle. As rarely as the buffer will ever be resized. (which is currently once) "clever" doesn't save us anything.

@roderickvd

Copy link
Copy Markdown
Member

Well, amen to that, I didn't like that clippy suggestion either. In this case couldn't we just do self.period_buffer = Vec::with_capacity(bytes_per_period) to simplify the three calls even further?

This way is less verbose, much more simple and less brittle.
@JasonLG1979

Copy link
Copy Markdown
Contributor Author

Well, amen to that, I didn't like that clippy suggestion either. In this case couldn't we just do self.period_buffer = Vec::with_capacity(bytes_per_period) to simplify the three calls even further?

Done.

@ashthespy

Copy link
Copy Markdown
Member

From the discussions previously over in #811, this undoes those "optimisations" correct? As it is deemed these micro optimisation are not really worth the extra cruft?

@JasonLG1979

Copy link
Copy Markdown
Contributor Author

From the discussions previously over in #811, this undoes those "optimisations" correct? As it is deemed these micro optimisation are not really worth the extra cruft?

Basically yes. This does 99.9% of what I was after, not reallocating if the buffer size doesn't change.

@roderickvd

Copy link
Copy Markdown
Member

Alright then, let's go for it.

@roderickvd
roderickvd merged commit 2c95645 into librespot-org:dev Oct 8, 2021
@JasonLG1979
JasonLG1979 deleted the simplify-buffer-resizing branch October 16, 2021 17:34
paulfariello pushed a commit to paulfariello/librespot that referenced this pull request Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants