RFR (S): 8067469: G1 ignores AlwaysPreTouch

[Erik Helin]

On 2014-12-16, Thomas Schatzl wrote:
> Hi Erik,
> 
>   thanks for the review.
> 
> On Tue, 2014-12-16 at 16:29 +0100, Erik Helin wrote:
> > On 2014-12-16, Thomas Schatzl wrote:
> > > Hi all,
> > > 
> > >   can I have reviews for the following small change?
> > > 
> > > It adds pre-touching of allocated memory in G1, which has been forgotten
> > > to be implemented when changing this code in JDK-8038423.
> > > 
> > > This change adds this and refactors the pre-touch code to a single
> > > method in the os class.
> > > 
> > > CC'ed runtime since it changes code in os.hpp/cpp.
> > > 
> > > CR:
> > > https://bugs.openjdk.java.net/browse/JDK-8067469
> > > Webrev:
> > > http://cr.openjdk.java.net/~tschatzl/8067469/webrev/
> > 
> > First of all, nice patch, I really like the introduction of
> > os::pretouch_memory. I have one question about the patch: the code you
> > removed in virtualspace.cpp did:
> > 
> >   *curr = 0;
> > 
> > whereas the new code you added in os::pretouch_memory does:
> > 
> >   char t = *p; *p = t;
> > 
> > Was this change intended or should os::pretouch_memory also write 0?
> > 
> 
> There is no code relying on particular values written. Pre-touch is done
> as part of committing new memory. So in effect, the code reads a zero,
> and writes it back.
> 
> It's just that there were two different pre-touching implementations and
> I chose one of them to factor out into the new method.

Ok, I see, thanks for the explanation. If it doesn't matter if we use
"*p = 0" or "char t = *p; *p = t", would you mind using "*p = 0"? I
immediately understand what "*p = 0" does whereas with
"char t = *p; *p = t" I have to think for a second :)

Thanks,
Erik

> Thanks,
>   Thomas
> 
>