[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8259778: Merge MutableSpace and ImmutableSpace [v3]
From:       Kim Barrett <kbarrett () openjdk ! java ! net>
Date:       2021-01-29 3:52:58
Message-ID: nHOtCw_JaPSV1KffsOymGKZlMIuOjG0U70UcXUR298g=.4cb34cb2-6868-4329-b6b9-b4f5ac984941 () github ! com
[Download RAW message or body]

On Thu, 28 Jan 2021 12:07:52 GMT, Thomas Schatzl <tschatzl@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@oracle.com) on \
> > [serviceability-dev](mailto:serviceability-dev@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


[prev in list] [next in list] [prev in thread] [next in thread]