RFR: 8259778: Merge MutableSpace and ImmutableSpace [v3]

[Kim Barrett]

On Thu, 28 Jan 2021 12:07:52 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> I assume that tier1-3 includes the SA tests :)
>> 
>> Looks good other than that nit.
>
> Hi David,
> 
>> _Mailing list message from [David Holmes](mailto:david.holmes at oracle.com) on [serviceability-dev](mailto:serviceability-dev at openjdk.java.net):_
>> 
>> On 28/01/2021 7:09 pm, Thomas Schatzl wrote:
>> 
>> > On Thu, 28 Jan 2021 05:13:57 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> > > > Please review this change which merges ImmutableSpace into MutableSpace,
>> > > > eliminating the former.  There were no interesting uses of ImmutableSpace,
>> > > > other than as the base class for MutableSpace.  The name ImmutableSpace is
>> > > > kind of a misnomer given that usage.
>> > > > Testing:
>> > > > mach5 tier1-3
>> > > 
>> > > src/hotspot/share/gc/parallel/mutableSpace.hpp line 47:
>> > > > 45: //
>> > > > 46: // Invariant: bottom() <= top() <= end()
>> > > > 47: // top() is inclusive and end() is exclusive.
>> > > 
>> > > If end() is exclusive then shouldn't the invariant be `< end()`?
>> > 
>> > I also think that top() is also exclusive as in other collectors.
>> > @dholmes-ora : e.g. bottom == top == end means the space is empty. These two lines are not disagreeing with each other.
>> 
>> If one is exclusive and one is inclusive then I don't see how they can
>> be equal, as that implies they are then both inclusive and exclusive at
>> the same time. ?? If end() is exclusive then I would expect an empty
>> space to be one where bottom and end are adjacent, not coincident.
>> 
>> Cheers,
>> David
> 
> The original comment about top() being inclusive is wrong. top() is also exclusive like in all other collectors as stated elsewhere in my review comment. My "also" in "I also think that top() is also exclusive as in other collectors." probably threw you off after re-reading it, which is wrong. Sorry. 
> 
> Maybe some examples help:
> 
> bottom = 200, top = 200, end = 200 is an "empty" space (i.e. is of size zero). Whether that empty space is "free" or "fully allocated" or both or neither is another question :)
> 
> bottom = 200, top = 200, end = 201 contains one word and is (completely) free (not allocated into at all).
> 
> bottom = 200, top = 201, end = 201 contains one word and is full(y allocated).
> 
> Top/end are exclusive, and bottom inclusive as does the code assume from what I can tell by quickly looking at it. Still the invariant is bottom <= top <= end in all cases.
> 
> Thanks,
>   Thomas

Thanks @tschatzl , @sspitsyn , and @dholmes-ora for reviews.

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

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