RFR: 8242901: Duplicate PSYoung/OldGen max size functions

[Kim Barrett]

> On May 11, 2020, at 1:26 PM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> 
> 
> 
> On 2020-05-11 18:43, Kim Barrett wrote:
>>> On May 11, 2020, at 5:15 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>>> 
>>> Looks good.
>>> 
>>> I think I would have preferred to get rid of the _gen infix, since it's redundant.
>>> 
>>> _init_gen_size => _init_size
>>> _min_gen_size => _min_size
>>> _max_gen_size => _max_size
>>> 
>>> min_gen_size() => min_size()
>>> max_size() => max_size()
>>> 
>>> I'll leave it up to you to decide if you want to do that change.
>> I thought about doing that earlier, and revisited the question because
>> of your comment. There are a number of min/max_mumble_size and mumble_size
>> names, so that I think an unadorned min/max_size ends up being somewhat
>> unhelpful.
> 
> 
> There's no other max/min in PSOldGen/PSYoungGen, so I don't understand what you have been looking at.
> 
> All I see is that a lot of places where the word gen is mentioned twice. For example:
> old_gen()->max_gen_size
> 
> which I think makes perfectly sense as:
> old_gen()->max_size()

max_survivor_size, max_eden_size, max_young_size, and other are all
names that appear in various places in the code, some in proximity to
the name max_gen_size. When browsing some of the code, I found the
extra bit of information somewhat helpful.

>> While I was looking at that I noticed a few places that were being
>> inconsistent with nearby code by directly accessing _min/max_gen_size
>> rather than using the accessor functions.  I tidied those up.
> OK. I tend to move class-internal code in the other direction and remove calls to getters.

I think it depends. In this particular case, I don't think it matters
much now, since further abstraction seems unlikely. So I went with the
substantial majority pre-existing form. I wonder though whether any of
the inconsistencies might have anything to do with bugs prior to the
UseAdaptiveGCBoundary removal. Not that I'm planning to go back and
figure that out.

>> New webrevs:
>> full: https://cr.openjdk.java.net/~kbarrett/8242901/open.01/
>> incr: https://cr.openjdk.java.net/~kbarrett/8242901/open.01.inc/
> 
> 
> OK.

Thanks.