On Wed, 18 Feb 2026 13:48:06 GMT, Stefan Karlsson <[email protected]> wrote:
>> I have a bunch of patches to remove some code duplication, redundant setting >> of fields, unneeded dispatch code. >> >> I've tested this with tier1-3. >> >> I'm opening this up as one PR, depending on #2033, but it could be split up >> into multiple dependent PRs. It could be good to look at each commit >> individually when making a first pass of over this PR. >> >> I'll add a guide to the individual commits as a comment after the summary. > > Stefan Karlsson has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 24 commits: > > - Merge remote-tracking branch 'valhalla/lworld' into > 8378086_obj_array_klass_code_deduplication > - Add temporary assert > - Restore ObjArrayKlass private > - Remove create_element_klass_array_name duplication > - Remove ObjArrayKlass::oop_verify_on implementation > - Remove ObjArrayKlass::oop_size > - Remove ObjArrayKlass::copy_array > - Fix incorrect reference to RefArrayKlass:: > - Deduplicate array klass code > - Fixes from Stefan's comments > - ... and 14 more: > https://git.openjdk.org/valhalla/compare/646c2f93...24bd1bb1 This looks really good! There's a tiny bit that will be a minor difference than in openjdk that would be better changed there with array dimensions, but I can approve this now. Also, I don't see any reason for history or help triaging any potential bugs to break this up. src/hotspot/share/oops/arrayKlass.cpp line 133: > 131: } > 132: > 133: Symbol* ArrayKlass::create_element_klass_array_name(Klass* > element_klass, JavaThread* current) { Should this be in ObjArrayKlass instead and removed here? It's not needed by typeArrayKlass right? src/hotspot/share/oops/arrayKlass.hpp line 69: > 67: // The constructor with the Symbol argument does the real array > 68: // initialization, the other is a dummy > 69: ArrayKlass(int n, Symbol* name, KlassKind kind, ArrayProperties props); I see why you did this now. The field dimension is in ArrayKlass not ObjArrayKlass. You should make this minor change in mainline, then have it merged in here. src/hotspot/share/oops/flatArrayKlass.cpp line 215: > 213: > 214: // Check destination > 215: if (!d->is_flatArray() && !d->is_refArray()) { There's an equivalent !is_refined_objArray() that can be used. ------------- Marked as reviewed by coleenp (Committer). PR Review: https://git.openjdk.org/valhalla/pull/2117#pullrequestreview-3896169557 PR Comment: https://git.openjdk.org/valhalla/pull/2117#issuecomment-4004874896 PR Review Comment: https://git.openjdk.org/valhalla/pull/2117#discussion_r2889734722 PR Review Comment: https://git.openjdk.org/valhalla/pull/2117#discussion_r2889718879 PR Review Comment: https://git.openjdk.org/valhalla/pull/2117#discussion_r2889746351
