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.

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

fix IB in fifoToOwnedArrayList #21404

Merged
merged 1 commit into from
Sep 24, 2024
Merged

Conversation

matklad
Copy link
Contributor

@matklad matklad commented Sep 14, 2024

memcpy requires non-overlapping arguments.

fifo.realign() handles this case correctly and tries to provide an optimized implementation.

This probably wasn't hit in practice, as, in a typical usage, fifo's head is not advanced.

memcpy requires non-overlapping arguments.

fifo.realign() handles this case correctly and tries to provide an
optimized implementation.

This probably wasn't hit in practice, as, in a typical usage, fifo's
head is not advanced.
Copy link
Contributor

@daurnimator daurnimator left a comment

Choose a reason for hiding this comment

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

I don't think you need the if (fifo.head != 0) check: realign should be cheap enough to do unconditionally.

@matklad
Copy link
Contributor Author

matklad commented Sep 16, 2024

Hm, if I am reading the code correctly, head==0 would go into this branch:

mem.copyForwards(T, self.buf[0..self.count], self.buf[self.head..][0..self.count]);

Which is a (noop) for-loop over the entire data, which feels like it could be costly, especially given that head==0 is the common case actually.

@daurnimator
Copy link
Contributor

Which is a (noop) for-loop over the entire data, which feels like it could be costly, especially given that head==0 is the common case actually.

sorry you're right!

@andrewrk andrewrk merged commit ffd071f into ziglang:master Sep 24, 2024
10 checks passed
@andrewrk
Copy link
Member

Thank you.

This data structure looks like a nice candidate to participate in a fuzz test that does various equivalent things with different std lib containers and then asserts that they get the same results.

@matklad matklad deleted the matklad/fifocopyleft branch September 25, 2024 00:39
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