On Tue, 18 Aug 2026 07:52:36 GMT, David Holmes <[email protected]> wrote:
>> To align with all other definition orderings:
>>
>> 1. Add to hierarchy.
>> 2. Add to dictionary.
>>
>> No reason for why this site would have it the other way.
>
> Have you confirmed that? It may be the order doesn't matter, but it seems an
> unnecessary change to make unless you want to verify that.
Because this code lacks asserts, it is impossible to verify.
My argument is that it is wrong to publish and share a klass, using a
Dictionary, that is not in a "loaded" state. The comment (again, unfortunately
no asserts), in Dictionary::add_klass() says as much, although I grant "loaded
class" could be interpreted non-technically:
// Add a loaded class to the dictionary.
void Dictionary::add_klass(JavaThread* current, Symbol* class_name,
InstanceKlass* klass) {
Ideally, I would want the same triple repeated at all definition sites, if
possible:
1. JFR_ONLY(Jfr::on_definition(klass)) // has strong asserts that
!klass->is_loaded()
2. klass->add_to_hierarchy(THREAD); assert(klass->is_loaded(), "Must be in at
least loaded state"); // this is what makes a klass go to state "loaded"
3. dictionary->add_klass(THREAD, klass->name(), klass); // The klass is now in
"loaded" state and is published.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32218#discussion_r3802627243
