RFR: 8352075: Perf regression accessing fields [v28]

[Johan Sjölen]

On Tue, 10 Jun 2025 13:45:52 GMT, Radim Vansa <rvansa at openjdk.org> wrote:

>> This was done this way with a "Supplier" because this package would be useful for other Unsigned5 packed types.
>
> But then you'd need create an array of `Pair` in `create_search_table` and copy the data into that. You wouldn't need a Supplier at all, just pass the array and its length to the `fill` method. If you are worried about the virtual calls, I could do that.
> 
> Alternatively, we could replace virtual calls with templated `fill` method. In that case the Comparator should be a template method as well, since that one is even more perf-critical?

I think you misunderstand me, as what I'm proposing wouldn't requre creating an array in `create_search_table`.

I'm saying that you do this:

```c++
// Note: we require the supplier to provide the elements in the final order as we can't easily sort
// within this method - qsort() accepts only pure function as comparator.
void PackedTableBuilder::fill(u1* table, size_t table_length, Supplier &supplier) const {
  Pair<uint32_t, uint32_t> kvs[4];

  size_t offset = 0;
  size_t len_read = 0;
  while (len = supplier.next(kvs, 4)) {
    // len tells you how many of the full capacity of kvs was used.
  }
}


Now each call of  `Supplier::next` gives you up to 4 elements, quartering the amount of virtual calls necessary.

>Alternatively, we could replace virtual calls with templated fill method. In that case the Comparator should be a template method as well, since that one is even more perf-critical?

Sure, you can try that out.

To be clear, I am only asking: Are virtual calls expensive enough here that not having them, or amortizing their cost, something that makes the performance better?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24847#discussion_r2138061151