kernel qsort: use inlined min() implementation from libkern.h

[pfg]

Summary

Looking at the recent changes in qsort, it looks reasonable to use the
inlined versions in libkern.

Test Plan

Passes tinderbox build. Should this be imin() on min()?

[delphij]

What do we gain from the change?

Note that imin/imax is int, by the way, while the result is ptrdiff_t (abused size_t), and they are not always equivalent, so I think this change is technically wrong.

[pfg]

In D10945#226586, @delphij wrote:

What do we gain from the change?

Performance.. if we do them a lot it may be noticeable.

Note that imin/imax is int, by the way, while the result is ptrdiff_t (abused size_t), and they are not always equivalent, so I think this change is technically wrong.

I though char* were supposed to by int's sometimes unsigned. I noticed a subtraction there though.
There is a bunch of functions of the same family: perhaps ummin().

[delphij]

In D10945#226591, @pfg wrote:

In D10945#226586, @delphij wrote:

What do we gain from the change?

Performance.. if we do them a lot it may be noticeable.

Do you have data to support this claim? (e.g. I don't see why inline expansion is different from macro expansion in the resulting code).

Note that imin/imax is int, by the way, while the result is ptrdiff_t (abused size_t), and they are not always equivalent, so I think this change is technically wrong.

I though char* were supposed to by int's sometimes unsigned. I noticed a subtraction there though.
There is a bunch of functions of the same family: perhaps ummin().

[pfg]

In D10945#226592, @delphij wrote:

In D10945#226591, @pfg wrote:

In D10945#226586, @delphij wrote:

What do we gain from the change?

Performance.. if we do them a lot it may be noticeable.

Do you have data to support this claim? (e.g. I don't see why inline expansion is different from macro expansion in the resulting code).

Well, this was the idea when the functions were introduced (BSD44 I think): since the function already defined the types the comparisons can be optimized and the code will be inlined. However this really depends on the compiler, which may or may not inline it, and/or modern compilers may apply similar optimizations to the macro. So the claim is probably bogus if you try to measure it.

I don't know ... stack overflow and other forums carry such discussion extensively and are inconclusive.
FWIW, I recall Marcelo was working on a coccinelle script to find these type of replacements. Is this not an objective anymore?

[pfg]

After reading a bunch of pros and cons between inline vs. macros, I got to the conclusion that it doesn't really matter. at least not for this case, and not something I want to spend time benchmarking.

[araujo]

In D10945#226616, @pfg wrote:

In D10945#226592, @delphij wrote:

In D10945#226591, @pfg wrote:

In D10945#226586, @delphij wrote:

What do we gain from the change?

Performance.. if we do them a lot it may be noticeable.

Do you have data to support this claim? (e.g. I don't see why inline expansion is different from macro expansion in the resulting code).

Well, this was the idea when the functions were introduced (BSD44 I think): since the function already defined the types the comparisons can be optimized and the code will be inlined. However this really depends on the compiler, which may or may not inline it, and/or modern compilers may apply similar optimizations to the macro. So the claim is probably bogus if you try to measure it.

I don't know ... stack overflow and other forums carry such discussion extensively and are inconclusive.
FWIW, I recall Marcelo was working on a coccinelle script to find these type of replacements. Is this not an objective anymore?

I know you have abandoned this patch already, but I'd like to share my point of view.
Replace inline by macros has some benefits that is not only related with performance, of course each case need to be analysed, but one big benefit for me is 'code consistency' and also sometimes it makes easier to read the code as well.

So, performance is not the only and main point for these changes.

Best,

[pfg]

In D10945#228108, @araujo wrote:

...

I know you have abandoned this patch already, but I'd like to share my point of view.
Replace inline by macros has some benefits that is not only related with performance, of course each case need to be analysed, but one big benefit for me is 'code consistency' and also sometimes it makes easier to read the code as well.

So, performance is not the only and main point for these changes.

OK, let's assume the performance is the same. Although I prefer the lowercase, syntactically using MIN() or ummin() is not huge difference.

I think the point is in the type: the macro doesn't care about the types but with an inline a compiler may do some extra checking. If we get the types right then using the inline function is worthy as a replacement. I don't know this code well enough to guess which type would be best (the subtraction may be reducing the range, or it may become negative?). Perhaps coccinelle may figure it out for us ;).