On Wed, 29 Oct 2025 14:56:14 GMT, Leonid Mesnik <[email protected]> wrote:
>>> Ah, so _there_! I confused myself. This one is readable: the counter `0`
>>> means we can free. It would be even better if you did
>>> `inc_immutable_data_refcount()` and `dec_immutable_data_refcount()`, and
>>> did e.g.:
>>>
>>> ```
>>> if (dec_immutable_data_refcount() == 0) {
>>> os::free(_immutable_data);
>>> }
>>>
>>> int dec_immutable_data_refcount() {
>>> int refcount = get(...);
>>> assert(refcount > 0, "Must be positive");
>>> set(refcount - 1);
>>> return refcount - 1;
>>> }
>>> ```
>>>
>>> Because the next thing you know this would need to be replaced with Atomics
>>> a year later.
>>
>> I agree this makes the code cleaner.
>>
>> I replaced the getter and setter for the counter with
>> `init_immutable_data_ref_count`, `inc_immutable_data_ref_count`, and
>> `dec_immutable_data_ref_count`. I also shortened the counter name from
>> `immutable_data_references_counter` to `immutable_data_ref_count`
>>
>> I modified `NMethod.java` to calculate the offsets that same way as is done
>> in the JVM. I missed this in
>> [JDK-8369642](https://bugs.openjdk.org/browse/JDK-8369642)
>>
>> The last notable change is that I modified the [immutable data size
>> calculation](https://github.com/chadrako/jdk/blob/26bdc3ceb4ab9ad9cb9a4218bb87ce2d7546fa22/src/hotspot/share/code/nmethod.cpp#L1155)
>> to only include a reference counter if there is immutable data
>
> @chadrako The testing pass now.
@lmesnik please add link to testing in confidential comment in JBS
-------------
PR Comment: https://git.openjdk.org/jdk/pull/28008#issuecomment-3462693312
