On Wed, 17 Sep 2025 20:41:28 GMT, David Holmes <[email protected]> wrote:
>> Leonid Mesnik has updated the pull request incrementally with one additional >> commit since the last revision: >> >> updated after David's feedback > > src/hotspot/share/runtime/suspendResumeManager.hpp line 63: > >> 61: >> 62: // The specific 'set_suspended' implementation for self suspend. >> 63: void set_suspended_with_id(int64_t id, bool register_vthread_SR); > > Suggestion: > > // Sets the suspended state to `to`, applying to vthreads if > `register_vthread_SR` is true. > void set_suspended(bool to, bool register_vthread_SR); > > // Sets the suspended state to true, applying to vthreads if > `register_vthread_SR` is true. > // Applied to the thread with the given `id` and used when we can't extract > the thread oop safely. > void set_suspended_with_id(int64_t id, bool register_vthread_SR); > > These comments are far from perfect as it is actually hard to explain exactly > what `thread` is being operated on - if any! Really the comment ```// Sets the suspended state to `to`, applying to vthreads if `register_vthread_SR` is true.``` looks incorrect to me. One might decide that the method doesn't set suspend state if for vthreads if `register_vthread_SR` is false. While the method always set suspend and might register or not vthreads to support SuspendAllVirtualThreads. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27317#discussion_r2356783702
