[Tarantool-patches] [PATCH 04/11] txn: txn_commit_async -- drop redundant variable
gorcunov at gmail.com
Sat Nov 21 15:30:38 MSK 2020
On Sat, Nov 21, 2020 at 12:55:16AM +0100, Vladislav Shpilevoy wrote:
> Thanks for the patch!
Look, there must be a serious reason why you use temp variable instead
of direct access to the variable itself. They could be
1) The variable might be changed externally after you fetched it
into another place (which means you might to need a compiler
barrier as well but it depends, sometime even compiler help
is not enough and you need hw barrier).
2) The variable is immutable but its name is too long and you use
temp variable as an alias (which compiler most likely to optimize).
If code usage in this case is too big then this become unacceptable
and better redesign the code.
3) It simply happened this way.
The reason 1) and 2) are pretty usable all over the world. In this
particular code we've (3). So I'm asking you "why do you need a temp
When we've been reviewing the whole qsync series I already told
that we better should use direct access to the flags all the
time until there is a reson 1) or 2) to cache the value.
But we've been under the time pressure then and I simply closed
my eyes. Since I'm working on this code right now I hate with
a passion such aggressive caching for NO reason, this even
makes flags grepping more hard - instead of validating flags
access you need to walk two ways: find flag read operation
then find the use of a new variable... and I wonder do you
*really* thing this is normal practice?
More information about the Tarantool-patches