On 07/02/2017 20:34, Ed Maste wrote:
On 7 February 2017 at 10:30, Steven Hartland
<steven.hartl...@multiplay.co.uk> wrote:
All I'm suggesting is, while one could guess this may be a performance or
possibly a compatibility thing, the reason is not obvious, so a small piece
of detail on why the change was done should always be included.
For this one something like the following would be nice:
Switch fget_unlocked to atomic_fcmpset
Improve performance under contention by switching fget_unlocked to
use atomic_fcmpset.
I agree, and one of the key reasons to do this is so that there's this
tiny bit of context if someone later runs "git blame" or "svn
annotate" and discovers this change for the line containing
atomic_fcmpset. Comments containing "eliminate memory leak" or "remove
unused variable" have a self-evident reason, but I don't believe
that's true for "switch to atomic_fcmpset."
Repeating the "switch fget_unlocked to..." in the proposed commit
message above feels redundant to me though, and I would suggest:
| Switch fget_unlocked to atomic_fcmpset
|
| Improves performance under contention.
or just:
| Use atmoic_fcmpset to improve performance under contention
All those work for me as they clearly state why the change was made, so
I hope this is something we can try to improve moving forward :)
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
