RFR: 8314502: Change the comparator taking version of GrowableArray::find to be a template method [v7]

[John R Rose]

On Sun, 29 Oct 2023 07:51:48 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> @dholmes-ora Yes it helps avoid copying, especially if the copy constructor is non-trivial. And I think it is more idiomatic in C++ to use references here.
>
> Using a reference here leads to unnecessary overhead when `E` is small and
> trivially copyable, unless the predicate function is inlined. Pass by value is
> better in that case. Of course, as noted above, if `E` is "expensive" to copy
> or non-copyable then a reference is needed. Boost has this thing called
> `call_traits<E>::param_type` for this issue, but I don't recommend we copy
> that.
> 
> Idiomatic C++ makes the entire function a template parameter, as was suggested
> earlier in this PR. That dodges this question entirely, leaving the parameter
> passing decision to the predicate function where it belongs, rather than
> having it imposed by GrowableArray::find.  The find function just imposes the
> requirement that the predicate satisfies the appropriate constraints, e.g. it
> is callable on an element reference and returns convertible to bool.

I agree we should be using a template-typed function instead of a function pointer here.
I think a lot of our uses of function pointers in our code base would work better as template-typed args.
See for example the `grow` argument (of template type `GFN`) at this point:
https://github.com/openjdk/jdk/blob/d051f22284e7ccc288c658588f73da672d9bfacd/src/hotspot/share/utilities/unsigned5.hpp#L343C34-L343C34

In that case I cited, if the `grow` argument it were a function pointer instead of a template-typed function-like argument (Kim what’s the right term here? “functor”??), then performance and flexibility would be unacceptably harmed.

I think the `find` argument is just the same kind of thingy.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15418#discussion_r1376940244