RFR (M): 8208498: Put archive regions into a first-class HeapRegionSet

[Hohensee, Paul]

Hi Thomas,

LGTM except for the following.

Analogous to old_set_add() and old_set_remove(), you might add old_set_bulk_remove(), humongous_set_add(), humongous_set_bulk_remove(), archive_set_add(), and archive_set_remove() methods and use them instead of directly accessing _old_set, _humongous_set and _archive_set.

g1CollectedHeap.cpp:
       // We ignore humongous and archive regions, we're not tearing down the
       // humongous regions set.
=> 
       // We ignore humongous and archive regions, we're not tearing down the
       // humongous and archive region sets.

g1CollectedHeap.hpp:
    return (_old_set.length() + _archive_set.length() + _humongous_set.length()) * HeapRegion::GrainBytes;
=>
    return (old_regions_count() + archive_regions_count() + humongous_regions_count()) * HeapRegion::GrainBytes

heapRegionSet.hpp:
Can you use an enum instead of a set of "const int" for the ForxxxRegion constants? Or, you could just use HeapRegionType.

Thanks,

Paul

On 8/9/18, 3:55 PM, "hotspot-gc-dev on behalf of sangheon.kim at oracle.com" <hotspot-gc-dev-bounces at openjdk.java.net on behalf of sangheon.kim at oracle.com> wrote:

    Hi Thomas,
    
    On 8/8/18 2:28 AM, Thomas Schatzl wrote:
    > Hi all,
    >
    >    can I have reviews for this change that adds a seperate HeapRegionSet
    > for archive regions.
    >
    > This will simplify getting information about them in the future.
    >
    > CR:
    > https://bugs.openjdk.java.net/browse/JDK-8208498
    > Webrev:
    > http://cr.openjdk.java.net/~tschatzl/8208498/webrev/
    > Testing:
    > hs-tier1-4, jdk-tier1-3
    Looks good.
    
    If you are interested, g1HeapTransition.*pp files need copyright update.
    I don't need a new webrev for this.
    
    Thanks,
    Sangheon
    
    >
    > Thanks,
    >    Thomas