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

Investigate Thread Safety Of SharedDataHolder::hashValid #1091

Open
danieldresser-ie opened this issue Oct 20, 2020 · 0 comments
Open

Investigate Thread Safety Of SharedDataHolder::hashValid #1091

danieldresser-ie opened this issue Oct 20, 2020 · 0 comments

Comments

@danieldresser-ie
Copy link
Contributor

We noticed an issue that can be reproduced in Gaffer by removing the line tile->cachedTile->Object::hash(); from Display::channelData() and running InteractiveArnoldRenderPerformanceTest.testPerfWithBlur with GAFFER_HASHCACHE_MODE set to Checked. The validator reports hashes varying when they shouldn't, which John hypothesizes is due to the way that SharedDataHolder uses a non-threadsafe approach with the hashValid variable handling multiple accesses to the hash. We don't have hard evidence of this, but the fact that precomputing the hashes while holding a lock fixes it, strongly suggests this hypothesis is correct.

We should create a test case that demonstrates incorrect hashing behaviour under heavy thread load, introduce a spinlock to fix it properly, and then make sure that this doesn't impact overall performance too much of tools that rely on SharedDataHolder, like Gaffer.

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

No branches or pull requests

1 participant