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: Performance improvements #8299

Merged
merged 2 commits into from Aug 9, 2019

Conversation

JosJuice
Copy link
Member

@JosJuice JosJuice commented Aug 6, 2019

In the tests I've done, these changes make verification 50% to 100% faster.

StringFromFormat(Common::GetStringT("Content %08x is corrupt.").c_str(), content.id));
}
m_content_future = std::async(std::launch::async, [this, read_succeeded, content] {
if (!read_succeeded || !m_volume.CheckContentIntegrity(content, m_data, m_ticket))
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. read_succeeded can possibly be moved outside the future so it only executes conditionally.
  2. Other m_volume methods are protected by m_volume_mutex, I assume it's not needed here?

Copy link
Member Author

@JosJuice JosJuice Aug 6, 2019

Choose a reason for hiding this comment

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

  1. The reason I didn't do that was because then I'd need to duplicate the call to AddProblem (one inside the lambda and one outside the lambda). A read failing is a rare event, and a WAD file usually doesn't have more than 10 contents or so anyway, so this won't have any noticeable performance impact.
  2. It's only needed if the volume is reading data from the disk, which is not the case here since we pass in a vector with the data to verify. It's the same if you look at the two different calls to CheckBlockIntegrity.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM then

Source/Core/DiscIO/VolumeVerifier.cpp Outdated Show resolved Hide resolved
Source/Core/DiscIO/VolumeWii.cpp Outdated Show resolved Hide resolved
Source/Core/DiscIO/VolumeVerifier.cpp Outdated Show resolved Hide resolved
@stenzek stenzek merged commit 2df522d into dolphin-emu:master Aug 9, 2019
@JosJuice JosJuice deleted the volumeverifier-performance branch August 9, 2019 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants