Minchan Kim <minchan@xxxxxxxxxx> writes: > Hi Huang, > > On Tue, May 02, 2017 at 01:35:24PM +0800, Huang, Ying wrote: >> Hi, Minchan, >> >> Minchan Kim <minchan@xxxxxxxxxx> writes: >> >> > On Fri, Apr 28, 2017 at 09:35:37PM +0800, Huang, Ying wrote: >> >> In fact, during the test, I found the overhead of sort() is comparable >> >> with the performance difference of adding likely()/unlikely() to the >> >> "if" in the function. >> > >> > Huang, >> > >> > This discussion is started from your optimization code: >> > >> > if (nr_swapfiles > 1) >> > sort(); >> > >> > I don't have such fast machine so cannot test it. However, you added >> > such optimization code in there so I guess it's *worth* to review so >> > with spending my time, I pointed out what you are missing and >> > suggested a idea to find a compromise. >> >> Sorry for wasting your time and Thanks a lot for your review and >> suggestion! >> >> When I started talking this with you, I found there is some measurable >> overhead of sort(). But later when I done more tests, I found the >> measurable overhead is at the same level of likely()/unlikely() compiler >> notation. So you help me to find that, Thanks again! >> >> > Now you are saying sort is so fast so no worth to add more logics >> > to avoid the overhead? >> > Then, please just drop that if condition part and instead, sort >> > it unconditionally. >> >> Now, because we found the overhead of sort() is low, I suggest to put >> minimal effort to avoid it. Like the original implementation, >> >> if (nr_swapfiles > 1) >> sort(); > > It might confuse someone in future and would make him/her send a patch > to fix like we discussed. If the logic is not clear and doesn't have > measureable overhead, just leave it which is more simple/clear. Because the added code is minimal and cheap, I tend to keep it and add some comments to avoid confusion. For example, /* * Although nr_swapfiles isn't absolute correct, but the overhead of sort() * is so low that it isn't necessary to optimize further. */ >> >> Or, we can make nr_swapfiles more correct as Tim suggested (tracking >> the number of the swap devices during swap on/off). > > It might be better option but it's still hard to justify the patch > because you said it's hard to measure. Such optimiztion patch should > be from numbers. OK. Best Regards, Huang, Ying -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
