RFR: 8323634: Shenandoah: Document behavior of EvacOOM protocol [v5]

Aleksey Shipilev shade at openjdk.org
Thu Feb 15 16:40:55 UTC 2024


On Wed, 24 Jan 2024 17:53:39 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> The protocol for handling OOM during evacuation is subtle and critical for correct operation.  This PR does NOT change behavior.  It provides improved documentation of existing behavior.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix spelling error and mismatched parentheses.

Sorry, but I think it needs significantly more work.

1. Either split out the actual code changes from this PR, or rename it to avoid the impression that it is a docs only change.
2. Describe the high-level protocol in one global comment, and the idea for implementation there.
3. Do _not_ repeat any of these near the code. Code is the source of truth here, not the comments.

Consider this: hardly anyone would read more than 1 page of comments. We have to be terse and up to the point. I tried to rewrite the top-level comment more tersely here: [evac-protocol.txt](https://github.com/openjdk/jdk/files/14299573/evac-protocol.txt)

src/hotspot/share/gc/shenandoah/shenandoahEvacOOMHandler.hpp line 232:

> 230:   /**
> 231:    * Enter a protected evacuation path.
> 232:    *

I see no point repeating _both_ the high-level description of protocol, _and_ the implementation of the method itself here.

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

Changes requested by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17385#pullrequestreview-1883312877
PR Review Comment: https://git.openjdk.org/jdk/pull/17385#discussion_r1491300081


More information about the hotspot-gc-dev mailing list