On Fri, 6 Feb 2026 19:31:00 GMT, Coleen Phillimore <[email protected]> wrote:

>> Frederic Parain has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains 12 additional 
>> commits since the last revision:
>> 
>>  - Comments update
>>  - Rename new_default_refArray to new_refArray with overload
>>  - Fixes issues mentioned in reviews
>>  - Remove force_refarray and add array klass creation from ArrayDescription
>>  - Fix merge
>>  - Merge remote-tracking branch 'upstream/lworld' into refarray_factory
>>  - Revert foreign methods signature changes
>>  - Foreign API and other fixes
>>  - CI fixes
>>  - More fixes in serviceability code
>>  - ... and 2 more: 
>> https://git.openjdk.org/valhalla/compare/dbed07f5...bd5f33d0
>
> src/hotspot/share/cds/cdsProtectionDomain.cpp line 332:
> 
>> 330:   if (_shared_jar_manifests.resolve() == nullptr) {
>> 331:     oop sjm = oopFactory::new_refArray(
>> 332:         vmClasses::Jar_Manifest_klass(), size, 
>> ArrayKlass::ArrayProperties::DEFAULT, CHECK);
> 
> Since there have been several of these added, I would like to see a new 
> oopFactory::new_default_refArray(), or even an overload of new_refArray 
> without the DEFAULT properties.  Internally in the JVM, they should all have 
> default properties, so only called from some Java-facing api would they not 
> be default.

Overload method added.

> src/hotspot/share/ci/ciArray.cpp line 66:
> 
>> 64:     {
>> 65:       if (ary->is_refArray()) {
>> 66:         assert(ary->is_objArray(), "");
> 
> Is this not given by the type system so unnecessary to assert?

Fixed.

> src/hotspot/share/ci/ciArray.cpp line 76:
> 
>> 74:         JavaThread* THREAD = CompilerThread::current();
>> 75:         oop elem = flatary->obj_at(index, THREAD);
>> 76:         if (HAS_PENDING_EXCEPTION) {
> 
> This is odd. The compiler cannot create exceptions because it can't call Java 
> code (nor can it allocate?).  Should this be CATCH instead?

The calls to element_value_impl() are guarded by VM_ENTRY_MARK, so this code is 
executed when the CompilerThread is _in_vm.
In case of a pending exception, the code clears the exception and returns an 
invalid constant, delegating the handling of the situation to the caller, which 
is free to opt for a different approach or to simply bail out.
This behavior was discussed with the compiler team.

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2033#discussion_r2784202562
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2033#discussion_r2784204843
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2033#discussion_r2775893435

Reply via email to