8227226: Segmented array clearing for ZGC

[Sciampacone, Ryan]

1) Yes this was a conscious decision.  There was discussion on determining the optimal point for breakup but given the existing sizes this seemed sufficient.  This doesn't preclude the ability to go down that path if its deemed absolutely necessary.  The path for more complex decisions is now available.
2) Agree 
3) I'm not clear here.  Do you mean effectively going direct to ZHeap and bypassing the single function PinAllocating?  Agree.  Otherwise I'll ask you to be a bit clearer.
4) Agree
5) I initially had the exact same reaction but I played around with a few other versions (including breaking up initialization points between header and body to get the desired results) and this ended up looking correct.  I'll try mixing in the mem clearer function again (fresh start) to see if it looks any better.

On 7/8/19, 5:49 AM, "Per Liden" <per.liden at oracle.com> wrote:

    Hi Ryan,
    
    A few general comments:
    
    1) It looks like this still only work for large pages?
    2) The log_info stuff should be removed.
    3) I'm not a huge fan of single-use utilities like PinAllocating, at 
    least not when, IMO, the alternative is more straight forward and less code.
    4) Please make locals const when possible.
    5) Duplicating _do_zero looks odd. Injecting a "mem clearer", similar to 
    what Stefans original patch did, seems worth exploring.
    
    cheers,
    /Per
    
    (Btw, I'm on vacation so I might not be super-responsive to emails)
    
    On 2019-07-08 12:42, Erik Österlund wrote:
    > Hi Ryan,
    > 
    > This looks good in general. Just some stylistic things...
    > 
    > 1) In the ZGC project we like the letter 'Z' so much that we put it in 
    > front of everything we possibly can, including all class names.
    > 2) We also explicitly state things are private even though it's 
    > bleedingly obvious.
    > 
    > So:
    > 
    > 39 class PinAllocating {
    > 40 HeapWord* _mem;
    > 41 public: -> 39 class ZPinAllocating { 40 private:    41 HeapWord* _mem;
    >    42
    >   41 public: I can be your sponsor and push this change for you. I don't 
    > think there is a need for another webrev for my small stylistic remarks, 
    > so I can just fix that before pushing this for you. On that note, I'll 
    > add me and StefanK to the contributed-by section as we all worked out 
    > the right solution to this problem collaboratively. I have run through 
    > mach5 tier1-5, and found no issues with this patch. Thanks, /Erik
    > 
    > On 2019-07-05 17:18, Sciampacone, Ryan wrote:
    >> http://cr.openjdk.java.net/~phh/8227226/webrev.00/
    >> https://bugs.openjdk.java.net/browse/JDK-8227226
    >>
    >> This patch introduces safe point checks into array clearing during 
    >> allocation for ZGC.  The patch isolates the changes to ZGC as (in 
    >> particular with the more modern collectors) the approach to 
    >> incrementalizing or respecting safe point checks is going to be 
    >> different.
    >>
    >> The approach is to keep the region holding the array in the allocating 
    >> state (pin logic) while updating the color to the array after checks.
    >>
    >> Can I get a review?  Thanks.
    >>
    >> Ryan
    >