On Tue, 28 Oct 2025 16:50:09 GMT, Aleksey Shipilev <[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

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

PR Comment: https://git.openjdk.org/jdk/pull/28008#issuecomment-3459206813

Reply via email to