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