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
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
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]
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
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
> 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
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
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
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
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
> 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
> 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
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()`.
>>
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_
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()
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
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/
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
> 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
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
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
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
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
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) {
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
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
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
> 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
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
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:
> 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
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
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
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
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
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
> 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
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,
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
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
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
> 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
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
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?
>>
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
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
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
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
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
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
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
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
52 matches
Mail list logo