Re: [Tarantool-patches] [PATCH 3/5] qsync: txn_commit_async -- drop redundant variable

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 10/07/2020 23:36, Cyrill Gorcunov wrote:
> On Fri, Jul 10, 2020 at 11:28:26PM +0200, Vladislav Shpilevoy wrote:
>> On 10/07/2020 23:10, Cyrill Gorcunov wrote:
>>> On Fri, Jul 10, 2020 at 10:35:50PM +0200, Vladislav Shpilevoy wrote:
>>>> Опять же. Патч технически корректен, но так же и бесполезен.
>>>> Его полезность сравнима с исправлением опечатки в каком-нибудь
>>>> не особо нужном комменте. Сори, если токсично звучит.
>>>>
>>>> Но не вижу в этом патче нужды.
>>>
>>> We've to allocate a variable which we simply don't need. Look,
>>> the former code reads the flag, puts it into variable then
>>> immediately read it and that's all :/ I think we should not
>>> spread @is_sync but read the flag as much as possible for
>>> better grep'ability.
>>
>> Do you have a proof that this change improves anything? That
>> the variable 'allocation' on the stack actually happens, and
>> costs even 1 ns?
> 
> You simply don't need it. I think the compiler will rip it off.
> Again, if you suspect that the code gonna be changed then ignoring
> the commit is perfectly fine. If not -- the useless variable is just
> a bad taste at minimum.

So we came to a conclusion that the patch has nothing to do with
perf. It is just subjective feeling of taste. Sorry, but I am not
buying it. If everyone in the team would rewrite every place in
the source code they consider 'bad taste', we would end up
rewriting each other's code 100% of work time.

Btw, talking of bad taste of mine - you yourself declare all
variables in the beginning like in C89. Isn't it bad taste? - I
don't know. Or is it a bad taste to do the same one-line change
in two neighbor functions in 2 separate commits like removal of

    (void) txn_limbo;

? This easily could be one commit, easier to review, and less commits
to skip in git log, when look for something. I don't know whether it
is a bad taste, because I don't have definition of bad taste. And you
either.

Even if I feel I would do something a bit different, it is certainly
not a reason for me now to go and "fix" all such places. Just because
I consider them not beautiful enough. There is a sanity border after
which refactoring becomes useless or even harmful when it comes to git
history and waste of time on that.