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@xxxxxxxxx> [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
