RFR: 8367989: Remove InstanceKlass::allocate_objArray

Stefan Karlsson stefank at openjdk.org
Fri Sep 19 08:17:47 UTC 2025


On Thu, 18 Sep 2025 18:27:14 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> This change removes InstanceKlass::allocate_objArray and has its caller call ObjArrayKlass::allocate_instance directly from oopFactory, like the other array allocations do.  See CR for more information why we should have this change.  I also removed element_klass_addr() and moved element_klass_offset() to be in a more logical place near element_klass() functions.  This upstreams a tiny valhalla diff.
> Tested with tier1-4.

src/hotspot/share/memory/oopFactory.cpp line 112:

> 110:   } else {
> 111:     ArrayKlass* ak = InstanceKlass::cast(klass)->array_klass(CHECK_NULL);
> 112:     return ObjArrayKlass::cast(ak)->allocate_instance(length, THREAD);

Before this change the two if/else branches had a symmetry that is lost with the proposed change. It makes you look at the code and wonder what the reason is for this asymmetry. Could the symmetry be retained by changing this to:


objArrayOop oopFactory::new_objArray(Klass* klass, int length, TRAPS) {
  ArrayKlass* ak;

  if (klass->is_array_klass()) {
    ak = ArrayKlass::cast(klass)->array_klass(CHECK_NULL);
  } else {
    ak = InstanceKlass::cast(klass)->array_klass(CHECK_NULL);
  }
  
  return ak->allocate_instance(length, THREAD);
}


Or if you dare to use a virtual call instead of the if branch:

objArrayOop oopFactory::new_objArray(Klass* klass, int length, TRAPS) {
  ArrayKlass* ak = klass->array_klass(CHECK_NULL);
  return ak->allocate_instance(length, THREAD);
}


If the virtual call is unwanted then we could add a new "faster" (unclear how much faster this actually is):

ArrayKlass* Klass::array_klass_fast(TRAPS) {
  ArrayKlass* ak;

  if (klass->is_array_klass()) {
    ak = ArrayKlass::cast(klass)->array_klass(CHECK_NULL);
  } else {
    ak = InstanceKlass::cast(klass)->array_klass(CHECK_NULL);
  }
 
  assert(ak == array_klass(), "The two functions should return the same result");
  return ak;
}
...
objArrayOop oopFactory::new_objArray(Klass* klass, int length, TRAPS) {
  ArrayKlass* ak = klass->array_klass_fast(CHECK_NULL);
  return ak->allocate_instance(length, THREAD);
}


(I've not compiled nor tested the above)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27372#discussion_r2362136758


More information about the hotspot-dev mailing list