RFR: 8253717: Relocate stack overflow code out of thread.hpp/cpp [v5]

Daniel D.Daugherty dcubed at openjdk.java.net
Wed Oct 7 16:03:13 UTC 2020


On Wed, 7 Oct 2020 15:22:31 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This change moves the significant amount of stack overflow related code (with ascii art!) out of thread files into a
>> new file.  Many of the functions are static functions and some go through JavaThread::_stack_overflow_state where
>> needed.   All functions are moved and not modified except for qualification.  I also added a delegating constructor to
>> JavaThread::JavaThread so reordered the assignments as initializers from JavaThread::initialize.
>> Tested with tier1-6 and builds on arm32, ppc, s390 and zero.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Added some const to StackOverflow declarations

Based on what you wrote here in the invite:

`All functions are moved and not modified except for qualification.`
`I also added a delegating constructor to JavaThread::JavaThread so reordered the assignments as initializers from
JavaThread::initialize.`

I didn't do my usual crawl through review of the old code and the new
code to make sure things "were the same". However, based on comments
posted by other folks there have been changes to the code after
movement, e.g., the change from `NULL` to `nullptr`. I'm going to
have to assume that other folks did a crawl through reviews.

Thumbs up.

src/hotspot/share/runtime/stackOverflow.hpp line 34:

> 32:
> 33: // StackOverflow handling is encapsulated in this class.  This class contains state variables
> 34: // for each JavaThread that implement stack overflow checking and guard page implementation.

This clause doesn't read quite right... Perhaps:

`// for each JavaThread that implement stack overflow checking and guard page functionality.`

The "that implement" and "guard page implementation." phrases in the same clause
just don't read right... Your call on making a change here.

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

Marked as reviewed by dcubed (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/522


More information about the hotspot-dev mailing list