RFR: 8189088: Add intrusive doubly-linked list utility [v3]

Kim Barrett kbarrett at openjdk.org
Sat Oct 7 21:02:14 UTC 2023


On Thu, 5 Oct 2023 10:18:18 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Kim Barrett has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   add IntrusiveListEntry::is_attached()
>
> src/hotspot/share/utilities/intrusiveList.hpp line 39:
> 
>> 37: class IntrusiveListImpl;
>> 38: 
>> 39: /**
> 
> Most long form comments I see in Hotspot do not use `/** */` and instead uses `//`, can the style be changed to this?

I used distinct commenting styles intentionally, in an attempt to distinguish
between "API documentation" and comments on the implementation.  I realize
this is relatively novel in HotSpot code.  I'm open to other ideas, but I
think that distinction is important here.

I don't think it's a good idea to skimp on the API documentation for a utility
like this, leaving questions to be answered by reading the code.  That leads
to people making assumptions based on the current implementation, making it
brittle.  Of course, someone might still inadvertently depend on some aspect
of the current implementation, but if there is clear API documentation then at
least a reviewer can call out violations.

Also, as several people have said to me privately, there's a lot of template
usage here that may not be familiar to all HotSpot devs. I think that makes
requiring someone to read the code to figure out how to use it a problem. I'm
actively striving to avoid requiring someone to read the code simply to use it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15896#discussion_r1349573793


More information about the hotspot-dev mailing list