On Thu, 8 Dec 2022 16:56:40 GMT, Afshin Zafari <d...@openjdk.org> wrote:

>> test of tier1-5 passed.
>
> Afshin Zafari has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8292741: Convert JvmtiTagMapTable to ResourceHashtable

I'm still seeing a few oddities in the code. Overall it is very hard in the PR 
to get a good sense of the conversion process, but it seems fairly reasonable. 
It is hard to see how all the different parts of the APIs connect e.g. to 
understand why some methods are returning boolean values that are always true.

Also please do not force-push.

src/hotspot/share/prims/jvmtiTagMap.cpp line 1262:

> 1260:   // and record the reference and tag value.
> 1261:   //
> 1262:   bool do_entry(JvmtiTagMapKey& key, jlong& value) {

I still see no point in this method returning a bool when it only ever returns 
true.

src/hotspot/share/prims/jvmtiTagMapTable.cpp line 114:

> 112:   JvmtiTagMapKey new_entry(obj);
> 113:   bool is_updated = _table.put(new_entry, tag) == false;
> 114:   assert(is_updated, "should be updated and not added");

This API is confusing. You have `add` and `replace` but in product mode an 
`add` can `replace` and a `replace` can `add`. It isn't clear if these should 
be allowed rare conditions or fatal errors.

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

PR: https://git.openjdk.org/jdk/pull/11288

Reply via email to