On Thu, 14 Nov 2024 00:50:51 GMT, Chen Liang <[email protected]> wrote:
>> Aleksey Shipilev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Review feedback: make sure trimming actually works, stylistic changes
>
> src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java line 236:
>
>> 234: static final class PhantomCleanableList {
>> 235: private static final int MIN_CAPACITY = 16;
>> 236: private final Object lock = new Object();
>
> Why use another lock object when identity of this list is not escaped
> elsewhere?
Mostly hygiene for not exposing the lock to external classes. But yeah, we can
simplify by just doing `synchronized`. I'll push the update soon.
> src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java line 311:
>
>> 309: // want to cause an immediate resize on next insertion.
>> 310: if ((size < arr.length / 4) && (size > MIN_CAPACITY)) {
>> 311: int newLen = ArraysSupport.newLength(size, 1, size);
>
> This code isn't immediately clear that it's trying to half the array size, as
> you are using the size computation method for array growth. Can you add a
> remark for so?
None of this is actually needed once we do smarter thing than relying on array
resizes (and copies). I'll push new implementation soon.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22043#discussion_r1841897671
PR Review Comment: https://git.openjdk.org/jdk/pull/22043#discussion_r1841899531
