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

[Wei Yang <richard.weiyang@gmail.com>]

On Fri, Sep 06, 2024 at 03:44:10AM +0000, Wei Yang wrote:
>On Thu, Sep 05, 2024 at 04:13:15PM -0400, Liam R. Howlett wrote:
>>* Wei Yang <richard.weiyang@gmail.com> [240904 10:53]:
>>> On Wed, Sep 04, 2024 at 07:58:19AM +0000, Wei Yang wrote:
>>> [...]
>>> >>It is only changing code for the sake of changing code.  And it looks
>>> >>like it will be slower, or the same speed if we are lucky.  I have to
>>> >>take time to verify things aren't slower or add subtle issues (maybe an
>>> >>RCU race) because the code looked similar.  It's just not worth it.
>>> >>
>>> >
>>> >I am trying to make the code more easy to read, but seems not helping.
>>> >
>>> >BTW, I found in mas_update_gap(), if (p_gap != max_gap), we would access
>>> >the first parent, parent's type and its gap twice. Once in mas_update_gap()
>>> >and once in mas_parent_gap().
>>> >
>>> >Do you think it worth a change to reduce one?
>>> >
>>> 
>>> Liam,
>>> 
>>> I am trying to understand what kind code change you don't like. 
>>
>>Any change that cleans things up and verifies the performance would be
>>acceptable, but your previous changes didn't do that so the burden is on
>>me to verify that your code isn't going to cause a regression.
>>
>
>Thanks for your reply. It contains much information which I am trying to
>digest. If I misunderstand, please let me know.
>
>>> 
>>> Is the following change worth?
>>
>>Not like it is right now, but this is worth fixing.
>>
>>Test using the tools/test/radix-tree/maple.c  Look in that file for
>>BENCH defines at the top, and examples of the benchmarking.
>>
>
>I guess I could run them by comment out those #define at the beginning of
>lib/test_maple_tree.c?
>
>I have comment out one of it, what I get is:
>
>    TEST STARTING
>    
>    maple_tree: 80000597 of 80000597 tests passed
>
>My question is how we leverage this test case? From the output itself
>I just see all passed, but I am not sure it breaks the benchmark or not.
>
>>Before you submit anything, run the testing to make sure it all passes.
>>
>
>Yes, I make and run ./maple for each change.
>
>>If you are changing anything for cleanup/optimisation, then write a
>>benchmark that will test the runtime, add that before your change and
>>run it in both before/after with the results.
>>
>
>I guess is to add a #define BENCH_XXX with related function and call it in
>maple_tree_seed(), right?
>
>>Look also into the perf tool to see what is going on and where your time
>>is spent, then you can avoid optimising something that's not worth
>>optimising.
>>
>
>This is use the perf tool on the new added benchmark test?
>
>I am lack of the experience of perf tool. I may need to spend some time to use
>it. Usually we begin with 'perf record ./maple', right?
>

Liam,

If you have some time, would you mind telling me the correct way to use the
benchmark?

So that I can do the correct verification as you expected.

-- 
Wei Yang
Help you, Help me