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

2024-07-26 Thread Vladimir Ivanov
On Fri, 26 Jul 2024 15:13:06 GMT, Andrew Haley wrote: >> This patch expands the use of a hash table for secondary superclasses >> to the interpreter, C1, and runtime. It also adds a C2 implementation >> of hashed lookup in cases where the superclass isn't known at compile >> time. >> >> HotSpot

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

2024-07-26 Thread Vladimir Ivanov
On Fri, 26 Jul 2024 15:13:06 GMT, Andrew Haley wrote: >> This patch expands the use of a hash table for secondary superclasses >> to the interpreter, C1, and runtime. It also adds a C2 implementation >> of hashed lookup in cases where the superclass isn't known at compile >> time. >> >> HotSpot

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

2024-07-26 Thread Andrew Haley
On Fri, 26 Jul 2024 18:39:27 GMT, Vladimir Ivanov 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]

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

2024-07-26 Thread Vladimir Ivanov
On Fri, 26 Jul 2024 15:13:06 GMT, Andrew Haley wrote: >> This patch expands the use of a hash table for secondary superclasses >> to the interpreter, C1, and runtime. It also adds a C2 implementation >> of hashed lookup in cases where the superclass isn't known at compile >> time. >> >> HotSpot

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

2024-07-26 Thread Andrew Haley
On Thu, 25 Jul 2024 23:31:21 GMT, Vladimir Ivanov wrote: > Thanks! The patch looks good, except there was one failure observed during > testing with the latest patch [1]. It does look related to the latest changes > you did in > [54050a5](https://github.com/openjdk/jdk/pull/19989/commits/54050

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

2024-07-26 Thread Andrew Haley
> This patch expands the use of a hash table for secondary superclasses > to the interpreter, C1, and runtime. It also adds a C2 implementation > of hashed lookup in cases where the superclass isn't known at compile > time. > > HotSpot shared runtime > -- > > Building hashed s

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

2024-07-25 Thread Vladimir Ivanov
On Thu, 25 Jul 2024 13:56:34 GMT, Andrew Haley wrote: >> Thanks, now I see that `Class::isInstance(Object)` is backed by >> `Runtime1::is_instance_of()` which uses `oopDesc::is_a()` to do the job. >> >> If it turns out to be performance critical, the intrinsic implementation >> should be rewri

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

2024-07-25 Thread Vladimir Ivanov
On Thu, 25 Jul 2024 16:05:49 GMT, Andrew Haley wrote: >> This patch expands the use of a hash table for secondary superclasses >> to the interpreter, C1, and runtime. It also adds a C2 implementation >> of hashed lookup in cases where the superclass isn't known at compile >> time. >> >> HotSpot

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

2024-07-25 Thread Vladimir Ivanov
On Thu, 25 Jul 2024 16:05:49 GMT, Andrew Haley wrote: >> This patch expands the use of a hash table for secondary superclasses >> to the interpreter, C1, and runtime. It also adds a C2 implementation >> of hashed lookup in cases where the superclass isn't known at compile >> time. >> >> HotSpot

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

2024-07-25 Thread Andrew Haley
On Thu, 25 Jul 2024 16:05:49 GMT, Andrew Haley wrote: >> This patch expands the use of a hash table for secondary superclasses >> to the interpreter, C1, and runtime. It also adds a C2 implementation >> of hashed lookup in cases where the superclass isn't known at compile >> time. >> >> HotSpot

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

2024-07-25 Thread Andrew Haley
> This patch expands the use of a hash table for secondary superclasses > to the interpreter, C1, and runtime. It also adds a C2 implementation > of hashed lookup in cases where the superclass isn't known at compile > time. > > HotSpot shared runtime > -- > > Building hashed s

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

2024-07-25 Thread Andrew Haley
> This patch expands the use of a hash table for secondary superclasses > to the interpreter, C1, and runtime. It also adds a C2 implementation > of hashed lookup in cases where the superclass isn't known at compile > time. > > HotSpot shared runtime > -- > > Building hashed s

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

2024-07-25 Thread Andrew Haley
On Wed, 24 Jul 2024 19:09:06 GMT, Vladimir Ivanov wrote: >>> > Also also, Klass::is_subtype_of() is used for C1 runtime. >>> >>> Can you elaborate, please? >> >> Sorry, that was rather vague. In C1-compiled code, the Java method >> `Class::isInstance(Object)`calls `Klass::is_subtype_of()`. >>

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

2024-07-24 Thread Vladimir Ivanov
On Wed, 24 Jul 2024 16:14:47 GMT, Andrew Haley wrote: >>> I suspect that Klass::search_secondary_supers() won't be inlinined in such >>> case. >> >> That's true, but it's true of every other function in that file. Is it not >> deliberate? > > FYI, somewhat related: AArch64 GCC inlines `lookup_

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

2024-07-24 Thread Vladimir Ivanov
On Wed, 24 Jul 2024 09:03:12 GMT, Andrew Haley wrote: >>> Also also, Klass::is_subtype_of() is used for C1 runtime. >> >> Can you elaborate, please? What I'm seeing in >> `Runtime1::generate_code_for()` for `slow_subtype_check` is a call into >> `MacroAssembler::check_klass_subtype_slow_path()

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

2024-07-24 Thread Andrew Haley
On Wed, 24 Jul 2024 15:51:26 GMT, Andrew Haley wrote: >>> What happens when users include `klass.hpp`, but not `klass.inline.hpp`? >>> How does it affect generated code? >>> >>> I suspect that `Klass::search_secondary_supers()` won't be inlinined in >>> such case. >> >> That is true. I can't

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

2024-07-24 Thread Andrew Haley
On Wed, 24 Jul 2024 14:29:09 GMT, Andrew Haley wrote: > I suspect that Klass::search_secondary_supers() won't be inlinined in such > case. That's true, but it's true of every other function in that file. Is it not deliberate? - PR Review Comment: https://git.openjdk.org/jdk/pull/

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

2024-07-24 Thread Andrew Haley
On Tue, 23 Jul 2024 19:00:02 GMT, Vladimir Ivanov wrote: > What happens when users include `klass.hpp`, but not `klass.inline.hpp`? How > does it affect generated code? > > I suspect that `Klass::search_secondary_supers()` won't be inlinined in such > case. That is true. I can't tell from thi

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

2024-07-24 Thread Andrew Haley
> This patch expands the use of a hash table for secondary superclasses > to the interpreter, C1, and runtime. It also adds a C2 implementation > of hashed lookup in cases where the superclass isn't known at compile > time. > > HotSpot shared runtime > -- > > Building hashed s

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

2024-07-24 Thread Andrew Haley
On Tue, 23 Jul 2024 19:14:57 GMT, Vladimir Ivanov wrote: > > Also also, Klass::is_subtype_of() is used for C1 runtime. > > Can you elaborate, please? Sorry, that was rather vague. In C1-compiled code, the Java method `Class::isInstance(Object)`calls `Klass::is_subtype_of()`. In general, I fi

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

2024-07-23 Thread Vladimir Ivanov
On Mon, 22 Jul 2024 17:19:46 GMT, Andrew Haley wrote: >> This patch expands the use of a hash table for secondary superclasses >> to the interpreter, C1, and runtime. It also adds a C2 implementation >> of hashed lookup in cases where the superclass isn't known at compile >> time. >> >> HotSpot

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

2024-07-23 Thread Vladimir Ivanov
On Mon, 22 Jul 2024 16:36:25 GMT, Andrew Haley wrote: >>> Alternatively, `Klass::is_subtype_of()` can unconditionally perform linear >>> search over secondary_supers array. >>> >>> Even though I very much like to see table lookup written in C++ >>> (accompanying heavily optimized platform-spec

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

2024-07-23 Thread Vladimir Ivanov
On Mon, 22 Jul 2024 17:19:46 GMT, Andrew Haley wrote: >> This patch expands the use of a hash table for secondary superclasses >> to the interpreter, C1, and runtime. It also adds a C2 implementation >> of hashed lookup in cases where the superclass isn't known at compile >> time. >> >> HotSpot

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

2024-07-23 Thread Vladimir Ivanov
On Mon, 22 Jul 2024 16:45:06 GMT, Andrew Haley wrote: >> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 4810: >> >>> 4808: Label* >>> L_success, >>> 4809: Label* >>> L_failure) {

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

2024-07-23 Thread Vladimir Ivanov
On Mon, 22 Jul 2024 14:00:35 GMT, Andrew Haley wrote: >> Also, `num_extra_slots == 0` check is redundant. > >> Since `secondary_supers` are hashed unconditionally now, is >> `interfaces->length() <= 1` check still needed? > > I don't think so, no. Our incoming `transitive_interfaces` is formed

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

2024-07-23 Thread Vladimir Ivanov
On Mon, 22 Jul 2024 14:16:05 GMT, Andrew Haley wrote: >> src/hotspot/share/oops/klass.cpp line 175: >> >>> 173: if (secondary_supers()->at(i) == k) { >>> 174: if (UseSecondarySupersCache) { >>> 175: ((Klass*)this)->set_secondary_super_cache(k); >> >> Does it make sense to asse

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

2024-07-23 Thread Vladimir Ivanov
On Mon, 22 Jul 2024 14:56:31 GMT, Andrew Haley wrote: >> src/hotspot/share/oops/klass.inline.hpp line 117: >> >>> 115: } >>> 116: >>> 117: inline bool Klass::search_secondary_supers(Klass *k) const { >> >> I see you moved `Klass::search_secondary_supers` in `klass.inline.hpp`, but >> I'm not

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

2024-07-22 Thread Andrew Haley
> This patch expands the use of a hash table for secondary superclasses > to the interpreter, C1, and runtime. It also adds a C2 implementation > of hashed lookup in cases where the superclass isn't known at compile > time. > > HotSpot shared runtime > -- > > Building hashed s

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

2024-07-22 Thread Andrew Haley
On Mon, 22 Jul 2024 16:50:47 GMT, Andrew Haley wrote: >> This patch expands the use of a hash table for secondary superclasses >> to the interpreter, C1, and runtime. It also adds a C2 implementation >> of hashed lookup in cases where the superclass isn't known at compile >> time. >> >> HotSpot

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

2024-07-22 Thread Andrew Haley
On Thu, 11 Jul 2024 23:57:27 GMT, Vladimir Ivanov wrote: >> Andrew Haley has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Review comments >> - Review comments > > src/hotspot/cpu/x86/macroAssembler_x86.cpp line 4810: > >> 4808:

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

2024-07-22 Thread Andrew Haley
> This patch expands the use of a hash table for secondary superclasses > to the interpreter, C1, and runtime. It also adds a C2 implementation > of hashed lookup in cases where the superclass isn't known at compile > time. > > HotSpot shared runtime > -- > > Building hashed s

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

2024-07-22 Thread Andrew Haley
On Mon, 22 Jul 2024 15:03:12 GMT, Andrew Haley wrote: >> Alternatively, `Klass::is_subtype_of()` can unconditionally perform linear >> search over secondary_supers array. >> >> Even though I very much like to see table lookup written in C++ >> (accompanying heavily optimized platform-specific

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

2024-07-22 Thread Andrew Haley
On Thu, 18 Jul 2024 20:11:03 GMT, Vladimir Ivanov wrote: > Alternatively, `Klass::is_subtype_of()` can unconditionally perform linear > search over secondary_supers array. > > Even though I very much like to see table lookup written in C++ (accompanying > heavily optimized platform-specific Ma

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

2024-07-22 Thread Andrew Haley
On Thu, 11 Jul 2024 23:07:43 GMT, Vladimir Ivanov wrote: >> Andrew Haley has updated the pull request incrementally with four additional >> commits since the last revision: >> >> - Review feedback >> - Review feedback >> - Review feedback >> - Cleanup check_klass_subtype_fast_path for AArch

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

2024-07-22 Thread Andrew Haley
On Fri, 5 Jul 2024 22:30:09 GMT, Vladimir Ivanov wrote: >> Andrew Haley has updated the pull request incrementally with four additional >> commits since the last revision: >> >> - Review feedback >> - Review feedback >> - Review feedback >> - Cleanup check_klass_subtype_fast_path for AArch6

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

2024-07-22 Thread Andrew Haley
On Thu, 11 Jul 2024 22:53:42 GMT, Vladimir Ivanov wrote: >> src/hotspot/share/oops/instanceKlass.cpp line 1410: >> >>> 1408: return nullptr; >>> 1409: } else if (num_extra_slots == 0) { >>> 1410: if (num_extra_slots == 0 && interfaces->length() <= 1) { >> >> Since `secondary_supers` a

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

2024-07-19 Thread Andrew Haley
> This patch expands the use of a hash table for secondary superclasses > to the interpreter, C1, and runtime. It also adds a C2 implementation > of hashed lookup in cases where the superclass isn't known at compile > time. > > HotSpot shared runtime > -- > > Building hashed s

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

2024-07-18 Thread Vladimir Ivanov
On Thu, 18 Jul 2024 20:07:14 GMT, Vladimir Ivanov wrote: >> I think not. It'd complicate C++ runtime for no useful reason. > > On the other hand, if `-XX:-UseSecondarySupersTable` is intended solely for > diagnostic purposes, then handling all possible execution modes uniformly is > preferable,

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

2024-07-18 Thread Vladimir Ivanov
On Wed, 17 Jul 2024 17:15:32 GMT, Andrew Haley wrote: >> src/hotspot/share/oops/klass.inline.hpp line 122: >> >>> 120: return true; >>> 121: >>> 122: bool result = lookup_secondary_supers_table(k); >> >> Should `UseSecondarySupersTable` affect `Klass::search_secondary_supers` as >> well

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

2024-07-18 Thread Vladimir Ivanov
On Thu, 18 Jul 2024 16:35:16 GMT, Andrew Haley wrote: >> On a second thought the following setter may be the culprit: >> >> void Klass::set_secondary_supers(Array* secondaries) { >> assert(!UseSecondarySupersTable || secondaries == nullptr, ""); >> set_secondary_supers(secondaries, SECONDARY

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

2024-07-18 Thread Vladimir Ivanov
On Thu, 18 Jul 2024 16:40:47 GMT, Andrew Haley wrote: >> src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp line 1040: >> >>> 1038: >>> 1039: // Secondary subtype checking >>> 1040: void lookup_secondary_supers_table(Register sub_klass, >> >> While browsing the code, I noticed that it's fa

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

2024-07-18 Thread Andrew Haley
> This patch expands the use of a hash table for secondary superclasses > to the interpreter, C1, and runtime. It also adds a C2 implementation > of hashed lookup in cases where the superclass isn't known at compile > time. > > HotSpot shared runtime > -- > > Building hashed s

Re: RFR: 8331341: secondary_super_cache does not scale well: C1 and interpreter

2024-07-18 Thread Andrew Haley
On Thu, 11 Jul 2024 23:39:11 GMT, Vladimir Ivanov wrote: >> This patch expands the use of a hash table for secondary superclasses >> to the interpreter, C1, and runtime. It also adds a C2 implementation >> of hashed lookup in cases where the superclass isn't known at compile >> time. >> >> HotSp

Re: RFR: 8331341: secondary_super_cache does not scale well: C1 and interpreter

2024-07-18 Thread Andrew Haley
On Thu, 11 Jul 2024 23:22:19 GMT, Vladimir Ivanov wrote: >> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 1433: >> >>> 1431: >>> 1432: // Don't check secondary_super_cache >>> 1433: if (super_check_offset.is_register() >> >> Do you see any effects from this particular change? >>

Re: RFR: 8331341: secondary_super_cache does not scale well: C1 and interpreter

2024-07-18 Thread Andrew Haley
On Wed, 17 Jul 2024 18:54:32 GMT, Vladimir Ivanov wrote: >> Now it starts to sound concerning... `Klass::set_secondary_supers()` >> initializes both `_secondary_supers` and `_bitmap` which implies that >> `Klass::is_subtype_of()` may be called on not yet initialized Klass. It >> that's the cas

Re: RFR: 8331341: secondary_super_cache does not scale well: C1 and interpreter

2024-07-17 Thread Vladimir Ivanov
On Wed, 17 Jul 2024 18:46:11 GMT, Vladimir Ivanov wrote: >> This is because the C++ runtime does secondary super cache lookups even >> before the bitmap has been calculated and the hash table sorted. In this >> case the bitmap is zero, so teh search thinks there are no secondary supers. >> Set

Re: RFR: 8331341: secondary_super_cache does not scale well: C1 and interpreter

2024-07-17 Thread Vladimir Ivanov
On Wed, 17 Jul 2024 17:13:49 GMT, Andrew Haley wrote: >> Another observation while browsing the code: `_secondary_supers_bitmap` >> would be a better name. (Same considerations apply to `_hash_slot`.) > > This is because the C++ runtime does secondary super cache lookups even > before the bitma

Re: RFR: 8331341: secondary_super_cache does not scale well: C1 and interpreter

2024-07-17 Thread Andrew Haley
On Fri, 5 Jul 2024 22:37:34 GMT, Vladimir Ivanov wrote: >> This patch expands the use of a hash table for secondary superclasses >> to the interpreter, C1, and runtime. It also adds a C2 implementation >> of hashed lookup in cases where the superclass isn't known at compile >> time. >> >> HotSpo

Re: RFR: 8331341: secondary_super_cache does not scale well: C1 and interpreter

2024-07-17 Thread Andrew Haley
On Thu, 11 Jul 2024 23:47:51 GMT, Vladimir Ivanov wrote: >> src/hotspot/share/oops/klass.cpp line 284: >> >>> 282: // which doesn't zero out the memory before calling the constructor. >>> 283: Klass::Klass(KlassKind kind) : _kind(kind), >>> 284:_bitmap(SECONDARY_S

Re: RFR: 8331341: secondary_super_cache does not scale well: C1 and interpreter

2024-07-11 Thread Vladimir Ivanov
On Thu, 11 Jul 2024 23:17:10 GMT, Vladimir Ivanov wrote: >> This patch expands the use of a hash table for secondary superclasses >> to the interpreter, C1, and runtime. It also adds a C2 implementation >> of hashed lookup in cases where the superclass isn't known at compile >> time. >> >> HotSp

Re: RFR: 8331341: secondary_super_cache does not scale well: C1 and interpreter

2024-07-11 Thread Vladimir Ivanov
On Tue, 2 Jul 2024 14:52:09 GMT, Andrew Haley wrote: > This patch expands the use of a hash table for secondary superclasses > to the interpreter, C1, and runtime. It also adds a C2 implementation > of hashed lookup in cases where the superclass isn't known at compile > time. > > HotSpot shared

RFR: 8331341: secondary_super_cache does not scale well: C1 and interpreter

2024-07-05 Thread Andrew Haley
This patch expands the use of a hash table for secondary superclasses to the interpreter, C1, and runtime. It also adds a C2 implementation of hashed lookup in cases where the superclass isn't known at compile time. HotSpot shared runtime -- Building hashed secondary tables is