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

Daniel D.Daugherty dcubed at openjdk.java.net
Tue Oct 6 16:47:11 UTC 2020


On Tue, 6 Oct 2020 12:13:00 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.

Really nice refactoring and cleanup. Thumbs up.

Any idea whether the change in compilation unit will have any performance effects?

src/hotspot/os/linux/os_linux.cpp line 2034:

> 2032:   if (!_stack_is_executable) {
> 2033:     for (JavaThreadIteratorWithHandle jtiwh; JavaThread *jt = jtiwh.next(); ) {
> 2034:       StackOverflow* sto = jt->stack_overflow_state();

So why use `sto` here when you use `overflow_state` above?
The GitHub review UI makes this difference more obvious than webrev...

Update: In other source files, you use `overflow_state`.

src/hotspot/share/runtime/stackOverflow.cpp line 2:

> 1: /*
> 2:  * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.

You have this copyright as a range of years while stackOverflow.hpp is just 2020.

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

> 35:   friend class JavaThread;
> 36:  public:
> 37:   // State of the stack guard pages for this thread.

The "this thread" part no longer reads as well...
I don't have a suggested rewording...

src/hotspot/share/runtime/thread.cpp line 2954:

> 2952:
> 2953: void JavaThread::frames_do(void f(frame*, const RegisterMap* map)) {
> 2954:   // ignore is there is no stack

typo - s/is there/if there/

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

Marked as reviewed by dcubed (Reviewer).

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


More information about the hotspot-dev mailing list