Skip to content
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.

Sign up for GitHub

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

RingDelayBufferTest: refactor includes and span creation #10843

Merged
merged 3 commits into from
Sep 1, 2022
Merged

RingDelayBufferTest: refactor includes and span creation #10843

merged 3 commits into from
Sep 1, 2022

Conversation

davidchocholaty
Copy link
Contributor

The PR just improves a little bit the RingDelayBufferTest. The unused include is removed and the multiple span creation from one SampleBuffer is avoided.

@davidchocholaty
RingDelayBufferTest: remove unused include
4579d2e 
The commit removes unused include of the "util/sample.h" based
on the previous changes.
@davidchocholaty
RingDelayBufferTest: avoid multiple span creation
e1c6e8a 
This commit avoids multiple span creation from the one sample buffer
(exactly SampleBuffer). Instead of that, the span is created once
and stored in the variable.
Swiftb0y
Swiftb0y reviewed Aug 30, 2022
View reviewed changes
Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

while you're at it, you can try to eliminate all the superfluous mixxx::spanutil::spanFromPtrLen(..., numSamples) calls as well.

@Swiftb0y
Copy link
Member

Swiftb0y commented Aug 30, 2022 •
edited
Loading

though fyi, optimizing for constructions of std::span is not really worth much effort. at least not when the goal is performance. std::span is a very, very cheap type to construct.

@davidchocholaty
Copy link
Contributor Author

though fyi, optimizing for calls to std::span is not really worth much effort. at least not when the goal is performance. std::span is a very, very cheap type to construct.

Okay, thank you for informing me. It just didn't make much sense to me to duplicate the same operation multiple times (but TBH I haven't known how span is a cheap operation).

@davidchocholaty
Copy link
Contributor Author

while you're at it, you can try to eliminate all the superfluous mixxx::spanutil::spanFromPtrLen(..., numSamples) calls as well.

All multiple used spans should be deduplicated now. Although I kept the just once used span as were because it didn't make much sense to me to store them in a variable.

@davidchocholaty
RingDelayBufferTest: eliminate spanFromPtrLen
9044dd8 
This commit avoids multiple span creation from the input buffer.
Instead of that, the span is created once and stored in the variable.
Swiftb0y
Swiftb0y approved these changes Aug 30, 2022
View reviewed changes
Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

lgtm. waiting for ci. thanks

@Swiftb0y Swiftb0y merged commit b9a2577 into mixxxdj:main Sep 1, 2022
@ronso0 ronso0 added this to the 2.4.0 milestone Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Swiftb0y Swiftb0y Swiftb0y approved these changes

Assignees
No one assigned
Labels
code quality
Projects
None yet
Milestone
2.4.0
Development

Successfully merging this pull request may close these issues.
3 participants
@davidchocholaty @Swiftb0y @ronso0