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

Avoid computing file hashes concurrently #10

Merged
merged 1 commit into from Mar 11, 2018

Conversation

Projects
None yet
2 participants
@asarium
Copy link
Member

commented Mar 9, 2018

Computing the hashes concurrently decreases overall hashing performance
since the OS has to do a lot of seek operations on the disk to allow all
hashing operations to operate in sequence. This is especially harmful on
HDDs.

This uses a standard Java lock to ensure that only onc hash is computed
at a time which improved hash performance immensely for me.

@Goober5000

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2018

Excellent observation! That hadn't occurred to me, but you're absolutely right. Thank you.

However, I wonder if it would be better to add the lock within IOUtils.computeHash (specifically, surrounding the FileInputStream operations). This is because mod patching, which also uses the hash function, could be running concurrently.

@asarium asarium force-pushed the asarium:fix/concurrentHashes branch from b4969ac to c3b1af6 Mar 9, 2018

@asarium

This comment has been minimized.

Copy link
Member Author

commented Mar 9, 2018

I changed to code so that it locks the computeHash operations instead.

@Goober5000

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2018

Looks good, but can you use the aligned brace style? This would keep it consistent with the rest of the codebase.

Avoid computing file hashes concurrently
Computing the hashes concurrently decreases overall hashing performance
since the OS has to do a lot of seek operations on the disk to allow all
hashing operations to operate in sequence. This is especially harmful on
HDDs.

This uses a standard Java lock to ensure that only onc hash is computed
at a time which improved hash performance immensely for me.

@asarium asarium force-pushed the asarium:fix/concurrentHashes branch from c3b1af6 to 5dbd32c Mar 11, 2018

@asarium

This comment has been minimized.

Copy link
Member Author

commented Mar 11, 2018

Fixed.

@Goober5000 Goober5000 merged commit 0db9a19 into scp-fs2open:master Mar 11, 2018

@asarium asarium deleted the asarium:fix/concurrentHashes branch Mar 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.