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

FreeQueue: automated test coverage #388

Merged
merged 4 commits into from
Aug 14, 2024
Merged

FreeQueue: automated test coverage #388

merged 4 commits into from
Aug 14, 2024

Conversation

abhayporwals
Copy link
Collaborator

@abhayporwals abhayporwals commented Jul 23, 2024 •
edited by hoch
Loading

  • Test cases for FreeQueue Class

Fixes #347

@abhayporwals abhayporwals added the gsoc24 Google Summer of Code 2024 label Jul 23, 2024
@abhayporwals abhayporwals self-assigned this Jul 23, 2024
@abhayporwals abhayporwals changed the title Added Test Cases For FreeQueue Class FreeQueue: automated test coverage Jul 23, 2024
@abhayporwals
Copy link
Collaborator Author

image

hoch
hoch reviewed Jul 23, 2024
View reviewed changes
Copy link
Member

@hoch hoch left a comment

Choose a reason for hiding this comment

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

Generally looks great! Mostly nits.

@mjwilson-google
Copy link
Collaborator

Let's merge #374 first, then make sure this class has the same amount of coverage.

mjwilson-google
mjwilson-google requested changes Jul 30, 2024
View reviewed changes
Copy link
Collaborator

@mjwilson-google mjwilson-google left a comment

Choose a reason for hiding this comment

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

Now that #374 is landed, let's take the best aspects of both PRs and apply them to both test suites.

mjwilson-google
mjwilson-google approved these changes Aug 9, 2024
View reviewed changes
Copy link
Collaborator

@mjwilson-google mjwilson-google left a comment

Choose a reason for hiding this comment

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

Looks good to me! Let's follow-up with another PR later to make both test suites more similar.

When I ran this., FreeQueue was significantly faster than FreeQueueSAB (about 2x). Is that what you were seeing too? We have to be a bit careful about reading too much into the results because this isn't a very accurate timing method though.

Please merge when you are ready.

@abhayporwals
Copy link
Collaborator Author

abhayporwals commented Aug 12, 2024 •
edited
Loading

Yeah Sure!

Hmm, I am not sure but yes there was some difference. I'll look into that.

Sure, we can merge this PR and I'll raise a new PR for extending the SAB TestCases and resolve some nits.
I think I'm not authorized to merge the PR. So, you have to do that 😅

@mjwilson-google mjwilson-google merged commit 5d579b8 into main Aug 14, 2024
5 checks passed
@mjwilson-google
Copy link
Collaborator

Sorry, didn't realize you weren't authorized!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoch hoch hoch left review comments

@mjwilson-google mjwilson-google mjwilson-google approved these changes

Assignees

@abhayporwals abhayporwals

Labels
gsoc24 Google Summer of Code 2024
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve RingBuffer
3 participants
@abhayporwals @mjwilson-google @hoch