RFR (round 5), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-12-03 Thread Roman Kennke
Round 5 of Shenandoah review includes:
- A fix for the @requires tag in TestFullGCCountTest.java. It should be
correct now. We believe the CMS @requires was also not quite right and
fixed it the same.

It reads now: Don't run this test if:
 - Actual GC set by harness is CMS *and* ExplicitGCInvokesConcurrent is
true, as set by harness
 - Actual GC set by harness is Shenandoah *and*
ExplicitGCInvokesConcurrent is not set false by harness (it's true by
default in Shenandoah, so this needs to be double-inverteed).

The @requires for CMS was wrong before (we think), because it would also
filter defaultGC + ExplicitGCInvokesConcurrent.

- Sorting of macros was fixed, as was pointed out by Per
- Some stuff was added to SA, as suggested by Jini
- Rebased on most current jdk/jdk code

Webrevs:
http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/05/

I also need reviews from GC reviewers for the CSR:
https://bugs.openjdk.java.net/browse/JDK-8214349

I already got reviews for:
[x] shared-runtime (coleenp)
[x] shared-compiler (kvn)

I got reviews for shared-build, but an earlier version, so maybe makes
sense to look over this again. Erik J, Magnus?

I still need approvals for:
[ ] shared-build  (kvn, erikj, ihse, pliden)
[ ] shared-gc (pliden, kbarrett)
[ ] shared-serviceability (jgeorge, pliden)
[ ] shared-tests  (lmesnik, pliden)
[ ] shenandoah-gc
[ ] shenandoah-tests


Thanks for your patience and ongoing support!

Cheers,
Roman


> Hi all,
> 
> here comes round 4 of Shenandoah upstreaming review:
> 
> This includes fixes for the issues that Per brought up:
> - Verify and gracefully reject dangerous flags combinations that
> disables required barriers
> - Revisited @requires filters in tests
> - Trim unused code from Shenandoah's SA impl
> - Move ShenandoahGCTracer to gc/shenandoah
> - Fix ordering of GC names in various files
> - Rename UINT64_FORMAT_HEX_W to UINT64_FORMAT_X_W
> 
> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/04/
> 
> Thanks everybody for taking time to review this!
> Roman
> 
>> Hello all,
>>
>> Thanks so far for all the reviews and support!
>>
>> I forgot to update the 'round' yesterday. We are in round 3 now :-)
>> Also, I fixed the numbering of my webrevs to match the review-round. ;-)
>>
>> Things we've changed today:
>> - We moved shenandoah-specific code out of .ad files into our own .ad
>> files under gc/shenandoah (in shenandoah-gc), how cool is that? This
>> requires an addition in build machinery though, see
>> make/hotspot/gensrc/GensrcAdlc.gmk (in shared-build).
>> - Improved zero-disabling and build-code-simplification as suggested by
>> Magnus and Per
>> - Cleaned up some leftovers in C2
>> - Improved C2 loop opts code by introducing another APIs in
>> BarrierSetC2. See the new APIs in shared-gc under BarrierSetC2.hpp.
>> - I don't see where it makes sense to put INCLUDE_SHENANDOAHGC guards now.
>> - We would all very much prefer to keep ShenandoahXYZNode names, as
>> noted earlier. This stuff is Shenandoah-specific, so let's just call it
>> that.
>> - Rehashed Shenandoah tests (formatting, naming, directory layout, etc)
>> - Rebased on jdk-12+22
>>
>> - Question: let us know if you need separate RFE for the new BSC2 APIs?
>>
>> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/03/
>>
>> Thanks,
>> Roman
>>
>>> Alright, we fixed:
>>> - The minor issues that Kim reported in shared-gc
>>> - A lot of fixes in shared-tests according to Leonid's review
>>> - Disabled SA heapdumping similar to ZGC as Per suggested
>>>
>>> Some notes:
>>> Leonid:  test/hotspot/jtreg/gc/TestFullGCCount.java was actually
>>> correct. The @requires there means to exclude runs with both CMS and
>>> ExplicitGCInvokesConcurrent at the same time, because that would be
>>> (expectedly) failing. It can run CMS, default GC and any other GC just
>>> fine. Adding the same clause for Shenandoah means the same, and filters
>>> the combination (+UseShenandoahGC)+(+ExplicitGCInvokesConcurrent). I
>>> made the condition a bit clearer by avoiding triple-negation.
>>>
>>> See:
>>> http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008457.html
>>>
>>> Per: Disabling the SA part for heapdumping makes 2 tests fail:
>>> - test/hotspot/jtreg/serviceability/sa/ClhsdbJhisto.java
>>> - test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java
>>>
>>> we filter them for Shenandoah now. I'm wondering: how do you get past
>>> those with ZGC?
>>>
>>> See:
>>> http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008466.html
>>>
>>> (Note to Leonid and tests reviewers: I'll add those related filters in
>>> next round).
>>>
>>> Vladimir: Roland integrated a bunch of changes to make loop* code look
>>> better. I can tell that we're not done with C2 yet. Can you look over
>>> the code and see what is ok, and especially what is not ok, so that we
>>> can focus our efforts on the relevant parts?
>>>
>>> Updated set of webrevs:
>>> http://cr.openjdk.java.

Re: RFR (round 5), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-12-04 Thread Leonid Mesnik
Hi

The shared tests changes looks good for me. Thank you for fixing and testing 
different combinations. 

Leonid

> On Dec 3, 2018, at 11:10 PM, Roman Kennke  wrote:
> 
> Round 5 of Shenandoah review includes:
> - A fix for the @requires tag in TestFullGCCountTest.java. It should be
> correct now. We believe the CMS @requires was also not quite right and
> fixed it the same.
> 
> It reads now: Don't run this test if:
> - Actual GC set by harness is CMS *and* ExplicitGCInvokesConcurrent is
> true, as set by harness
> - Actual GC set by harness is Shenandoah *and*
> ExplicitGCInvokesConcurrent is not set false by harness (it's true by
> default in Shenandoah, so this needs to be double-inverteed).
> 
> The @requires for CMS was wrong before (we think), because it would also
> filter defaultGC + ExplicitGCInvokesConcurrent.
> 
> - Sorting of macros was fixed, as was pointed out by Per
> - Some stuff was added to SA, as suggested by Jini
> - Rebased on most current jdk/jdk code
> 
> Webrevs:
> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/05/
> 
> I also need reviews from GC reviewers for the CSR:
> https://bugs.openjdk.java.net/browse/JDK-8214349
> 
> I already got reviews for:
> [x] shared-runtime (coleenp)
> [x] shared-compiler (kvn)
> 
> I got reviews for shared-build, but an earlier version, so maybe makes
> sense to look over this again. Erik J, Magnus?
> 
> I still need approvals for:
> [ ] shared-build  (kvn, erikj, ihse, pliden)
> [ ] shared-gc (pliden, kbarrett)
> [ ] shared-serviceability (jgeorge, pliden)
> [ ] shared-tests  (lmesnik, pliden)
> [ ] shenandoah-gc
> [ ] shenandoah-tests
> 
> 
> Thanks for your patience and ongoing support!
> 
> Cheers,
> Roman
> 
> 
>> Hi all,
>> 
>> here comes round 4 of Shenandoah upstreaming review:
>> 
>> This includes fixes for the issues that Per brought up:
>> - Verify and gracefully reject dangerous flags combinations that
>> disables required barriers
>> - Revisited @requires filters in tests
>> - Trim unused code from Shenandoah's SA impl
>> - Move ShenandoahGCTracer to gc/shenandoah
>> - Fix ordering of GC names in various files
>> - Rename UINT64_FORMAT_HEX_W to UINT64_FORMAT_X_W
>> 
>> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/04/
>> 
>> Thanks everybody for taking time to review this!
>> Roman
>> 
>>> Hello all,
>>> 
>>> Thanks so far for all the reviews and support!
>>> 
>>> I forgot to update the 'round' yesterday. We are in round 3 now :-)
>>> Also, I fixed the numbering of my webrevs to match the review-round. ;-)
>>> 
>>> Things we've changed today:
>>> - We moved shenandoah-specific code out of .ad files into our own .ad
>>> files under gc/shenandoah (in shenandoah-gc), how cool is that? This
>>> requires an addition in build machinery though, see
>>> make/hotspot/gensrc/GensrcAdlc.gmk (in shared-build).
>>> - Improved zero-disabling and build-code-simplification as suggested by
>>> Magnus and Per
>>> - Cleaned up some leftovers in C2
>>> - Improved C2 loop opts code by introducing another APIs in
>>> BarrierSetC2. See the new APIs in shared-gc under BarrierSetC2.hpp.
>>> - I don't see where it makes sense to put INCLUDE_SHENANDOAHGC guards now.
>>> - We would all very much prefer to keep ShenandoahXYZNode names, as
>>> noted earlier. This stuff is Shenandoah-specific, so let's just call it
>>> that.
>>> - Rehashed Shenandoah tests (formatting, naming, directory layout, etc)
>>> - Rebased on jdk-12+22
>>> 
>>> - Question: let us know if you need separate RFE for the new BSC2 APIs?
>>> 
>>> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/03/
>>> 
>>> Thanks,
>>> Roman
>>> 
 Alright, we fixed:
 - The minor issues that Kim reported in shared-gc
 - A lot of fixes in shared-tests according to Leonid's review
 - Disabled SA heapdumping similar to ZGC as Per suggested
 
 Some notes:
 Leonid:  test/hotspot/jtreg/gc/TestFullGCCount.java was actually
 correct. The @requires there means to exclude runs with both CMS and
 ExplicitGCInvokesConcurrent at the same time, because that would be
 (expectedly) failing. It can run CMS, default GC and any other GC just
 fine. Adding the same clause for Shenandoah means the same, and filters
 the combination (+UseShenandoahGC)+(+ExplicitGCInvokesConcurrent). I
 made the condition a bit clearer by avoiding triple-negation.
 
 See:
 http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008457.html
 
 Per: Disabling the SA part for heapdumping makes 2 tests fail:
 - test/hotspot/jtreg/serviceability/sa/ClhsdbJhisto.java
 - test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java
 
 we filter them for Shenandoah now. I'm wondering: how do you get past
 those with ZGC?
 
 See:
 http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008466.html
 
 (Note to Leonid and tests reviewers: I'll add those related filters in

Re: RFR (round 5), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-12-04 Thread Roman Kennke
Thanks, Leonid, for reviewing!

Roman


> Hi
> 
> The shared tests changes looks good for me. Thank you for fixing and testing 
> different combinations. 
> 
> Leonid
> 
>> On Dec 3, 2018, at 11:10 PM, Roman Kennke  wrote:
>>
>> Round 5 of Shenandoah review includes:
>> - A fix for the @requires tag in TestFullGCCountTest.java. It should be
>> correct now. We believe the CMS @requires was also not quite right and
>> fixed it the same.
>>
>> It reads now: Don't run this test if:
>> - Actual GC set by harness is CMS *and* ExplicitGCInvokesConcurrent is
>> true, as set by harness
>> - Actual GC set by harness is Shenandoah *and*
>> ExplicitGCInvokesConcurrent is not set false by harness (it's true by
>> default in Shenandoah, so this needs to be double-inverteed).
>>
>> The @requires for CMS was wrong before (we think), because it would also
>> filter defaultGC + ExplicitGCInvokesConcurrent.
>>
>> - Sorting of macros was fixed, as was pointed out by Per
>> - Some stuff was added to SA, as suggested by Jini
>> - Rebased on most current jdk/jdk code
>>
>> Webrevs:
>> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/05/
>>
>> I also need reviews from GC reviewers for the CSR:
>> https://bugs.openjdk.java.net/browse/JDK-8214349
>>
>> I already got reviews for:
>> [x] shared-runtime (coleenp)
>> [x] shared-compiler (kvn)
>>
>> I got reviews for shared-build, but an earlier version, so maybe makes
>> sense to look over this again. Erik J, Magnus?
>>
>> I still need approvals for:
>> [ ] shared-build  (kvn, erikj, ihse, pliden)
>> [ ] shared-gc (pliden, kbarrett)
>> [ ] shared-serviceability (jgeorge, pliden)
>> [ ] shared-tests  (lmesnik, pliden)
>> [ ] shenandoah-gc
>> [ ] shenandoah-tests
>>
>>
>> Thanks for your patience and ongoing support!
>>
>> Cheers,
>> Roman
>>
>>
>>> Hi all,
>>>
>>> here comes round 4 of Shenandoah upstreaming review:
>>>
>>> This includes fixes for the issues that Per brought up:
>>> - Verify and gracefully reject dangerous flags combinations that
>>> disables required barriers
>>> - Revisited @requires filters in tests
>>> - Trim unused code from Shenandoah's SA impl
>>> - Move ShenandoahGCTracer to gc/shenandoah
>>> - Fix ordering of GC names in various files
>>> - Rename UINT64_FORMAT_HEX_W to UINT64_FORMAT_X_W
>>>
>>> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/04/
>>>
>>> Thanks everybody for taking time to review this!
>>> Roman
>>>
 Hello all,

 Thanks so far for all the reviews and support!

 I forgot to update the 'round' yesterday. We are in round 3 now :-)
 Also, I fixed the numbering of my webrevs to match the review-round. ;-)

 Things we've changed today:
 - We moved shenandoah-specific code out of .ad files into our own .ad
 files under gc/shenandoah (in shenandoah-gc), how cool is that? This
 requires an addition in build machinery though, see
 make/hotspot/gensrc/GensrcAdlc.gmk (in shared-build).
 - Improved zero-disabling and build-code-simplification as suggested by
 Magnus and Per
 - Cleaned up some leftovers in C2
 - Improved C2 loop opts code by introducing another APIs in
 BarrierSetC2. See the new APIs in shared-gc under BarrierSetC2.hpp.
 - I don't see where it makes sense to put INCLUDE_SHENANDOAHGC guards now.
 - We would all very much prefer to keep ShenandoahXYZNode names, as
 noted earlier. This stuff is Shenandoah-specific, so let's just call it
 that.
 - Rehashed Shenandoah tests (formatting, naming, directory layout, etc)
 - Rebased on jdk-12+22

 - Question: let us know if you need separate RFE for the new BSC2 APIs?

 http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/03/

 Thanks,
 Roman

> Alright, we fixed:
> - The minor issues that Kim reported in shared-gc
> - A lot of fixes in shared-tests according to Leonid's review
> - Disabled SA heapdumping similar to ZGC as Per suggested
>
> Some notes:
> Leonid:  test/hotspot/jtreg/gc/TestFullGCCount.java was actually
> correct. The @requires there means to exclude runs with both CMS and
> ExplicitGCInvokesConcurrent at the same time, because that would be
> (expectedly) failing. It can run CMS, default GC and any other GC just
> fine. Adding the same clause for Shenandoah means the same, and filters
> the combination (+UseShenandoahGC)+(+ExplicitGCInvokesConcurrent). I
> made the condition a bit clearer by avoiding triple-negation.
>
> See:
> http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008457.html
>
> Per: Disabling the SA part for heapdumping makes 2 tests fail:
> - test/hotspot/jtreg/serviceability/sa/ClhsdbJhisto.java
> - test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java
>
> we filter them for Shenandoah now. I'm wondering: how do you get past
> those with ZGC?
>
> See:
> http://mail.ope

Re: RFR (round 5), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-12-06 Thread coleen . phillimore



http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/05/shenandoah-gc/src/hotspot/share/gc/shenandoah/vm_operations_shenandoah.cpp.html

Can you rename these to shenandoahVMOperations.hpp/cpp to match the 
newly agreed upon naming convention for this?


See 8214791: Consistently name gc files containing VM operations [Was: 
Re: RFR (S): 8214791: Rename vm_operations_g1* files to g1VMOperations*]


thanks,
Coleen

On 12/4/18 3:37 PM, Roman Kennke wrote:

Thanks, Leonid, for reviewing!

Roman



Hi

The shared tests changes looks good for me. Thank you for fixing and testing 
different combinations.

Leonid


On Dec 3, 2018, at 11:10 PM, Roman Kennke  wrote:

Round 5 of Shenandoah review includes:
- A fix for the @requires tag in TestFullGCCountTest.java. It should be
correct now. We believe the CMS @requires was also not quite right and
fixed it the same.

It reads now: Don't run this test if:
- Actual GC set by harness is CMS *and* ExplicitGCInvokesConcurrent is
true, as set by harness
- Actual GC set by harness is Shenandoah *and*
ExplicitGCInvokesConcurrent is not set false by harness (it's true by
default in Shenandoah, so this needs to be double-inverteed).

The @requires for CMS was wrong before (we think), because it would also
filter defaultGC + ExplicitGCInvokesConcurrent.

- Sorting of macros was fixed, as was pointed out by Per
- Some stuff was added to SA, as suggested by Jini
- Rebased on most current jdk/jdk code

Webrevs:
http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/05/

I also need reviews from GC reviewers for the CSR:
https://bugs.openjdk.java.net/browse/JDK-8214349

I already got reviews for:
[x] shared-runtime (coleenp)
[x] shared-compiler (kvn)

I got reviews for shared-build, but an earlier version, so maybe makes
sense to look over this again. Erik J, Magnus?

I still need approvals for:
[ ] shared-build  (kvn, erikj, ihse, pliden)
[ ] shared-gc (pliden, kbarrett)
[ ] shared-serviceability (jgeorge, pliden)
[ ] shared-tests  (lmesnik, pliden)
[ ] shenandoah-gc
[ ] shenandoah-tests


Thanks for your patience and ongoing support!

Cheers,
Roman



Hi all,

here comes round 4 of Shenandoah upstreaming review:

This includes fixes for the issues that Per brought up:
- Verify and gracefully reject dangerous flags combinations that
disables required barriers
- Revisited @requires filters in tests
- Trim unused code from Shenandoah's SA impl
- Move ShenandoahGCTracer to gc/shenandoah
- Fix ordering of GC names in various files
- Rename UINT64_FORMAT_HEX_W to UINT64_FORMAT_X_W

http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/04/

Thanks everybody for taking time to review this!
Roman


Hello all,

Thanks so far for all the reviews and support!

I forgot to update the 'round' yesterday. We are in round 3 now :-)
Also, I fixed the numbering of my webrevs to match the review-round. ;-)

Things we've changed today:
- We moved shenandoah-specific code out of .ad files into our own .ad
files under gc/shenandoah (in shenandoah-gc), how cool is that? This
requires an addition in build machinery though, see
make/hotspot/gensrc/GensrcAdlc.gmk (in shared-build).
- Improved zero-disabling and build-code-simplification as suggested by
Magnus and Per
- Cleaned up some leftovers in C2
- Improved C2 loop opts code by introducing another APIs in
BarrierSetC2. See the new APIs in shared-gc under BarrierSetC2.hpp.
- I don't see where it makes sense to put INCLUDE_SHENANDOAHGC guards now.
- We would all very much prefer to keep ShenandoahXYZNode names, as
noted earlier. This stuff is Shenandoah-specific, so let's just call it
that.
- Rehashed Shenandoah tests (formatting, naming, directory layout, etc)
- Rebased on jdk-12+22

- Question: let us know if you need separate RFE for the new BSC2 APIs?

http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/03/

Thanks,
Roman


Alright, we fixed:
- The minor issues that Kim reported in shared-gc
- A lot of fixes in shared-tests according to Leonid's review
- Disabled SA heapdumping similar to ZGC as Per suggested

Some notes:
Leonid:  test/hotspot/jtreg/gc/TestFullGCCount.java was actually
correct. The @requires there means to exclude runs with both CMS and
ExplicitGCInvokesConcurrent at the same time, because that would be
(expectedly) failing. It can run CMS, default GC and any other GC just
fine. Adding the same clause for Shenandoah means the same, and filters
the combination (+UseShenandoahGC)+(+ExplicitGCInvokesConcurrent). I
made the condition a bit clearer by avoiding triple-negation.

See:
http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008457.html

Per: Disabling the SA part for heapdumping makes 2 tests fail:
- test/hotspot/jtreg/serviceability/sa/ClhsdbJhisto.java
- test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java

we filter them for Shenandoah now. I'm wondering: how do you get past
those with ZGC?

See:
http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-N

Re: RFR (round 5), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-12-06 Thread Roman Kennke
Hi Coleen,

> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/05/shenandoah-gc/src/hotspot/share/gc/shenandoah/vm_operations_shenandoah.cpp.html
> 
> 
> Can you rename these to shenandoahVMOperations.hpp/cpp to match the
> newly agreed upon naming convention for this?
> 
> See 8214791: Consistently name gc files containing VM operations [Was:
> Re: RFR (S): 8214791: Rename vm_operations_g1* files to g1VMOperations*]

Doing so:
http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-December/008595.html

I will integrate this in next round of webrevs.

I expect the next will be the final round of webrevs. I got positive
reviews for all shared-* parts, I'm waiting for shenandoah-* reviews
from Shenandoah devs, plus Zhengyu+Thomas' TaskQueue stuff to arrive in
upstream jdk. The CSR for Shenandoah flags has been approved, and the
JEP should be moved to targeted JDK12 ~tomorrow. I intend/expect to push
Shenandoah during the next couple of days, unless somebody speaks up
until then :-)

Thanks,
Roman

> 
> thanks,
> Coleen
> 
> On 12/4/18 3:37 PM, Roman Kennke wrote:
>> Thanks, Leonid, for reviewing!
>>
>> Roman
>>
>>
>>> Hi
>>>
>>> The shared tests changes looks good for me. Thank you for fixing and
>>> testing different combinations.
>>>
>>> Leonid
>>>
 On Dec 3, 2018, at 11:10 PM, Roman Kennke  wrote:

 Round 5 of Shenandoah review includes:
 - A fix for the @requires tag in TestFullGCCountTest.java. It should be
 correct now. We believe the CMS @requires was also not quite right and
 fixed it the same.

 It reads now: Don't run this test if:
 - Actual GC set by harness is CMS *and* ExplicitGCInvokesConcurrent is
 true, as set by harness
 - Actual GC set by harness is Shenandoah *and*
 ExplicitGCInvokesConcurrent is not set false by harness (it's true by
 default in Shenandoah, so this needs to be double-inverteed).

 The @requires for CMS was wrong before (we think), because it would
 also
 filter defaultGC + ExplicitGCInvokesConcurrent.

 - Sorting of macros was fixed, as was pointed out by Per
 - Some stuff was added to SA, as suggested by Jini
 - Rebased on most current jdk/jdk code

 Webrevs:
 http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/05/

 I also need reviews from GC reviewers for the CSR:
 https://bugs.openjdk.java.net/browse/JDK-8214349

 I already got reviews for:
 [x] shared-runtime (coleenp)
 [x] shared-compiler (kvn)

 I got reviews for shared-build, but an earlier version, so maybe makes
 sense to look over this again. Erik J, Magnus?

 I still need approvals for:
 [ ] shared-buildĀ  (kvn, erikj, ihse, pliden)
 [ ] shared-gc (pliden, kbarrett)
 [ ] shared-serviceability (jgeorge, pliden)
 [ ] shared-testsĀ  (lmesnik, pliden)
 [ ] shenandoah-gc
 [ ] shenandoah-tests


 Thanks for your patience and ongoing support!

 Cheers,
 Roman


> Hi all,
>
> here comes round 4 of Shenandoah upstreaming review:
>
> This includes fixes for the issues that Per brought up:
> - Verify and gracefully reject dangerous flags combinations that
> disables required barriers
> - Revisited @requires filters in tests
> - Trim unused code from Shenandoah's SA impl
> - Move ShenandoahGCTracer to gc/shenandoah
> - Fix ordering of GC names in various files
> - Rename UINT64_FORMAT_HEX_W to UINT64_FORMAT_X_W
>
> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/04/
>
> Thanks everybody for taking time to review this!
> Roman
>
>> Hello all,
>>
>> Thanks so far for all the reviews and support!
>>
>> I forgot to update the 'round' yesterday. We are in round 3 now :-)
>> Also, I fixed the numbering of my webrevs to match the
>> review-round. ;-)
>>
>> Things we've changed today:
>> - We moved shenandoah-specific code out of .ad files into our own .ad
>> files under gc/shenandoah (in shenandoah-gc), how cool is that? This
>> requires an addition in build machinery though, see
>> make/hotspot/gensrc/GensrcAdlc.gmk (in shared-build).
>> - Improved zero-disabling and build-code-simplification as
>> suggested by
>> Magnus and Per
>> - Cleaned up some leftovers in C2
>> - Improved C2 loop opts code by introducing another APIs in
>> BarrierSetC2. See the new APIs in shared-gc under BarrierSetC2.hpp.
>> - I don't see where it makes sense to put INCLUDE_SHENANDOAHGC
>> guards now.
>> - We would all very much prefer to keep ShenandoahXYZNode names, as
>> noted earlier. This stuff is Shenandoah-specific, so let's just
>> call it
>> that.
>> - Rehashed Shenandoah tests (formatting, naming, directory layout,
>> etc)
>> - Rebased on jdk-12+22
>>
>> - Question: let us know if you need separate RFE for the

Re: RFR (round 5), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-12-07 Thread Aleksey Shipilev
On 12/4/18 8:10 AM, Roman Kennke wrote:
> Webrevs:
> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/05/
> 
> [ ] shenandoah-gc
> [ ] shenandoah-tests

These two parts look good.

-Aleksey



Re: RFR (round 5), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-12-07 Thread Roland Westrelin


> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/05/

C2 changes look good to me.

Roland.