On Thu, 15 Dec 2022 01:57:27 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> I agree that this is ambigious. The `jvmtiTagMap` calls `add/update` methods
>> of `jvmtiTagMapTable` which in turn calls `resourceHashTable` methods `put`
>> and `put_if_absent`.
>> `put` and `put_if_absent` can be used for both adding and updating and
>> reporting it in returned values.
>> `jvmtiTagMap` calls to `jvmtiTagMapTable` add/update DO NOT CARE about the
>> returned values. For these calls, it doesn't mater if an add (update) method
>> resulted in an update (add).
>> So which one of the following cases would be correct?
>> - Based on the `jvmtiTagMap` calls, let the add/update be void and ignore
>> the differences of what really happens in the table. Or
>> - Based on the `resourceHashTable` bool-valued `put` and `put_if_absent`
>> methods, let the add/update be bool-values and leave the decision on "what
>> really happened in the table" to the callers in `jvmtiTagMap`. (current
>> implementation)
>
> The issue is not the underlying RHT methods but the semantics of the
> `jvmtiTagMap` methods. If a call to add always expects to add then it should
> be a fatal error if it actually did an update as that indicates something is
> broken. Similarly if an update actually does an add.
The JvmtiTagMap code adds/updates and removes the entries like below in two
places which could probably be simplified, but I think that's outside the scope
of this RFE. I suggest removing the bool return from add, remove and update,
and add the assert(true) in the remove case. There's already an assert that
add/update happened in the other cases.
``` // if the object is not already tagged then we tag it
if (found_tag == 0) {
if (tag != 0) {
hashmap->add(o, tag);
} else {
// no-op
}
} else {
// if the object is already tagged then we either update
// the tag (if a new tag value has been provided)
// or remove the object if the new tag value is 0.
if (tag == 0) {
hashmap->remove(o);
} else {
hashmap->update(o, tag);
}
}
-------------
PR: https://git.openjdk.org/jdk/pull/11288
