On Wed, 8 Dec 2021 at 16:00, Conor McCarthy cmccarthy@codeweavers.com wrote:
Greatly improves performance in Control.
This doesn't tell someone reading the commit log all that much. What's "greatly"? Is that a 5% increase? Or perhaps 20%? 500%? Perhaps at least as important, what is the baseline? If it's running at 200 fps, and this takes of 2.5ms, that's going to double fps, yes; on the other hand, if it's running at 40 fps, and this takes of 10ms, that would be a fair bit more interesting, even if the percentage increase is a bit smaller. What hardware/driver/etc. is this on? Also, ideally that explanation would go into a comment next to the code checking for redundant copies, instead of into the commit message.
/* Shadow of the Tomb Raider and possibly other titles sometimes destroy
* and rewrite a descriptor in another thread while it is being copied. */
* and rewrite a descriptor in another thread while it is being copied.* We don't need to lock either descriptor for the update check. The purpose* of locking descriptors is not to make descriptor writes thread safe,* but only to prevent use-after-free of the vkd3d_view caused by a race* condition in the calling app. It's not important if the update check* falls out of date as it's their race condition, not ours. */- if (dst->magic == src->magic)
- {
The comment probably belongs to the block actually taking the lock more than the block checking for redundant updates.