Re: [PATCH 1/3] maple_tree: use ma_data_end() in mas_data_end()

Wei Yang
2024-09-04T00:15:31Z

On Tue, Sep 03, 2024 at 12:12:39PM -0400, Liam R. Howlett wrote:
>* Wei Yang <richard.weiyang@gmail.com> [240830 20:11]:
>> These two function share almost the same code and do the same thing.
>
>Please stop trying to optimise code like this. I don't have time to
>verify you are not making things worse and you haven't done any of the
>testing required.
>
>I went through with perf to optimise dereferences and static inline
>functions, so unless you are prepared to look at the generated code and
>actually benchmark, please stop sending patches that I need to verify
>like this myself.
>

Would you mind letting me know how could I verify the generated code and
benchmark? What you expect to do?

For example this one, these two functions share the same code. What's your
concern for this? The inline function expansion?

>>
>> Let's just call ma_data_end() in mas_data_end() to reduce duplicate
>> code.
>>
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> ---
>> lib/maple_tree.c | 22 ++++------------------
>> 1 file changed, 4 insertions(+), 18 deletions(-)
>>
>> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
>> index b7d747a7938e..85668246f944 100644
>> --- a/lib/maple_tree.c
>> +++ b/lib/maple_tree.c
>> @@ -1435,28 +1435,14 @@ static __always_inline unsigned char ma_data_end(struct maple_node *node,
>> */
>> static inline unsigned char mas_data_end(struct ma_state *mas)
>> {
>> - enum maple_type type;
>> - struct maple_node *node;
>> - unsigned char offset;
>> - unsigned long *pivots;
>> -
>> - type = mte_node_type(mas->node);
>> - node = mas_mn(mas);
>> - if (type == maple_arange_64)
>> - return ma_meta_end(node, type);
>> + enum maple_type type = mte_node_type(mas->node);
>> + struct maple_node *node = mas_mn(mas);
>> + unsigned long *pivots = ma_pivots(node, type);
>>
>> - pivots = ma_pivots(node, type);
>> if (unlikely(ma_dead_node(node)))
>> return 0;
>>
>> - offset = mt_pivots[type] - 1;
>> - if (likely(!pivots[offset]))
>> - return ma_meta_end(node, type);
>> -
>> - if (likely(pivots[offset] == mas->max))
>> - return offset;
>> -
>> - return mt_pivots[type];
>> + return ma_data_end(node, type, pivots, mas->max);
>> }
>>
>> /*
>> --
>> 2.34.1
>>
>>

--
Wei Yang
Help you, Help me