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

VolumeVerifier: Handle overlapping blocks more efficiently #8625

Open
wants to merge 2 commits into
base: master
from

Conversation

@JosJuice
Copy link
Member

JosJuice commented Feb 10, 2020

The performance gains of doing this aren't too important since you normally wouldn't run into any disc image that has overlapping blocks (which by extension means overlapping partitions), but this change also lets us get rid of things like VolumeVerifier's mutex that used to exist just for the sake of handling overlapping blocks.

JosJuice added 2 commits Aug 6, 2019
The performance gains of doing this aren't too important since you
normally wouldn't run into any disc image that has overlapping blocks
(which by extension means overlapping partitions), but this change also
lets us get rid of things like VolumeVerifier's mutex that used to
exist just for the sake of handling overlapping blocks.
This can't actually happen in practice due to how WAD files work,
but it's very easy to add support for thanks to the last commit,
so we might as well add support for it.

const u64 bytes_to_copy = std::min(m_excess_bytes, bytes_to_read);
if (bytes_to_copy > 0)
std::memcpy(data.data(), m_data.data() + m_data.size() - m_excess_bytes, bytes_to_copy);

This comment has been minimized.

Copy link
@CookiePLMonster

CookiePLMonster Feb 10, 2020

Contributor

Can destination and source overlap here?

This comment has been minimized.

Copy link
@JosJuice

JosJuice Feb 10, 2020

Author Member

No. The source is the class member m_data and the destination is the local variable data.

This comment has been minimized.

Copy link
@CookiePLMonster

CookiePLMonster Feb 10, 2020

Contributor

Bliiiiiiiind.

This comment has been minimized.

Copy link
@JosJuice

JosJuice Feb 10, 2020

Author Member

If it makes you feel any better, I originally wrote this commit in August and couldn't be bothered debugging a critical problem with it until today, and it turned out that part of the problem was that I had written m_data for the destination instead of data :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.