RFR: 8253717: Relocate stack overflow code out of thread.hpp/cpp [v3]
Coleen Phillimore
coleenp at openjdk.java.net
Wed Oct 7 14:45:22 UTC 2020
On Wed, 7 Oct 2020 06:37:57 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>>
>> fix comments
>
> src/hotspot/share/runtime/stackOverflow.hpp line 121:
>
>> 119: // (large addresses)
>> 120: //
>> 121:
>
> Nice ascii art :)
>
> I wish though it could communicate better the openness of the ranges. E.g.:
>
> +----------------+
> | | <-- stack_end()
> | red zone |
> | |
> +----------------+
> | | <-- red_zone_base()
> | yellow zone |
> | |
> ....
> | |
> +----------------+
> <-- stack_base()
>
> Maybe its just me but I always have to think a bit more here. With downward growing stacks normal range thinking is
> reversed wrt to openness, so stack_base() points outside the stack and stack_end() is in the stack. This is true for
> all base values - they point to locations outside the zone they base. Maybe that is clear to all others but it
> sometimes surprises me.
I just found this comment. I think the ascii art was added by @GoeLin. I just moved it. Your picture is upside down
but it sorta makes sense that the 'base' addresses point to the first address in the range, which is what I think they
do.
> src/hotspot/share/runtime/stackOverflow.hpp line 131:
>
>> 129: static size_t _stack_shadow_zone_size;
>> 130: public:
>> 131: static size_t stack_red_zone_size() {
>
> Naming: since you moved these formerly-Thread-methods into this enclosing class all the "_stack" and "stack" prefixes
> for members and methods are not needed anymore.
I don't want to rename these here.
> src/hotspot/share/runtime/stackOverflow.hpp line 141:
>
>> 139: _stack_red_zone_size = s;
>> 140: }
>> 141: address stack_red_zone_base() {
>
> could be const (as could a couple of others, I won't mark them individually)
The static member functions can't have const. Let me see if there are others.
-------------
PR: https://git.openjdk.java.net/jdk/pull/522
More information about the hotspot-dev
mailing list