RFR: 8253717: Relocate stack overflow code out of thread.hpp/cpp
Coleen Phillimore
coleenp at openjdk.java.net
Tue Oct 6 18:51:13 UTC 2020
On Tue, 6 Oct 2020 16:25:58 GMT, Daniel D. Daugherty <dcubed 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.
>
> 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`.
I'll change it to overflow_state. I should be consistent. Fixed.
> 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.
oops. fixed.
> 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...
// State of the stack guard pages for the containing thread.
?
> 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/
Fixed. That code block showed up in the diff since I moved the function just above the one that called it.
-------------
PR: https://git.openjdk.java.net/jdk/pull/522
More information about the hotspot-dev
mailing list