RFR: 8331341: secondary_super_cache does not scale well: C1 and interpreter [v9]

[Andrew Haley]

On Fri, 26 Jul 2024 18:39:27 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

> Oh, it comes as a surprise to me... I was under impression that the first thing hand-coded assembly variants do is check for `bitmap != SECONDARY_SUPERS_BITMAP_FULL`. At least, it was my recollection from working on [JDK-8180450](https://bugs.openjdk.org/browse/JDK-8180450). (And the initial version of the patch with the check in `Klass::lookup_secondary_supers_table()` and `_bitmap(SECONDARY_SUPERS_BITMAP_FULL)` only reassured me that's still the case.)
> 
> > The root cause of this bug is that we're not maintaining the invariant popcount(bitmap) == secondary_supers()->length().
> 
> The invariant holds only when `bitmap != SECONDARY_SUPERS_BITMAP_FULL`.

Yes, exactly so. That's what I intended to mean.

> It does help that even in case of non-hashed `secondary_supers` list `secondary_supers()->length() >= popcount(bitmap)`, but initial probing becomes much less efficient (a random probe followed by a full linear pass over secondary supers list).
> 
> Alternatively, all table lookups can be adjusted to start with `bitmap != SECONDARY_SUPERS_BITMAP_FULL` checks before probing the table. It does add a branch on the fast path (and slightly increases inlined snippet code size), but the branch is highly predictable and works on a value we need anyway.

True enough. I've been trying to move as much as I can out of the inlined code, though.

 > So, I would be surprised to see any performance effects from it. IMO it's easier to reason and more flexible: `SECONDARY_SUPERS_BITMAP_FULL == bitmap` simply disables all table lookups and unconditionally falls back to linear search.

I take your point. But that seems like it's a bit of a sledgehammer to crack a walnut, don't you think? Given that it's only necessary to fix a rare edge case. But I'm not going to be precious about this choice, I'll test for a full bitmap first if you prefer. It's only a couple of instructions, but they are in the fast path that is successful in almost 99% of cases.

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

PR Comment: https://git.openjdk.org/jdk/pull/19989#issuecomment-2253537338