[Tarantool-patches] [PATCH] relay: yield explicitly every N sent rows

[Serge Petrenko]

24.02.2021 01:30, Vladislav Shpilevoy пишет:
> Thanks for the patch!
Thanks for the reivew!

>>>> diff --git a/src/box/relay.cc b/src/box/relay.cc
>>>> index 1d8edf116..6d9269e1d 100644
>>>> --- a/src/box/relay.cc
>>>> +++ b/src/box/relay.cc
>>>> @@ -117,6 +117,11 @@ struct relay {
>>>>            * is passed by the replica on subscribe.
>>>>            */
>>>>           uint32_t id_filter;
>>>> +       /**
>>>> +        * How many rows has this relay sent to the replica. Used to yield once
>>>> +        * in a while when reading a WAL to unblock the event loop.
>>>> +        */
>>>> +       size_t row_cnt;
> Name is a bit ugly, because all the other members are not
> contractions. They use full words. But up to you.

No problem, changed to `row_count`.
>>> 1. But it is not a size of anything, right? Maybe make it
>>> int64_t then?
>> uint64_t, probably?
> Nope, int64_t. It is supposed to be 'faster'. Because it does
> not have defined overflow rules, and therefore the hardware does
> not need to handle it.
>
> But honestly, I didn't measure. For me it is more cargo cult. I
> just use signed integers where I can assuming that the hardware
> really may omit an instruction or so.
>
> Up to you.

Long story short, I'd like to leave it as is. Besides, we have an 
unsigned type
(size_t) in local recovery.

Ok, now I see what you meant.
I never thought of this, and brief googling showed no signs of a speedup
with signed arithmetic vs unsigned.

Actually, the standard says signed overflow is an undefined behaviour while
an unsigned overflow should result in a wrap (modulo 2^64 in our case).

Do you think this wrap may be costly on some architecture?
>
> The patch about power of 2 worked btw. This is how it looks now:
>
> 	andq   $0x7fff, %rcx             ; imm = 0x7FFF
>
> this is how it used to look:
>
> 	movl   $0x7d00, %ecx             ; imm = 0x7D00
> 	divq   %rcx

Yeah, that was a nice catch.

-- 
Serge Petrenko