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

[Jini George]

Hi Roman,

Thank you for making the changes. The SA portion looks good to me. One 
nit though:


In 
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/HeapSummary.java, 
in printGCAlgorithm(), does displaying the nbr of Parallel GC threads 
not make sense for Shenandoah (like it is for G1, ZGC, etc)?


Thank you,
Jini.


On 12/4/2018 12:57 AM, 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
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 

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

[Jini George]

Hi Roman,

A few comments on the SA changes:

==> Could you please add the following lines in 
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/HSDB.java from line 
1120 onwards to avoid the "[Unknown generation]" message with hsdb while 
displaying the Stack Memory for a mutator thread ?


else if (collHeap instanceof ShenandoahHeap) {
   ShenandoahHeap heap = (ShenandoahHeap) collHeap;
   anno = "ShenandoahHeap ";
   bad = false;
}

==> 
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/HeapSummary.java


The printGCAlgorithm() method would need to be updated to read in the 
UseShenandoahGC flag to avoid the default "Mark Sweep Compact GC" being 
displayed with jhsdb jmap -heap.


==> 
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/shared/GCName.java


Could you please add "Shenandoah" to the GCName enum list ?

==> 
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/shared/GCCause.java


Could you please update the GCCause enum values to include these:

_shenandoah_stop_vm,
_shenandoah_allocation_failure_evac,
_shenandoah_concurrent_gc,
_shenandoah_traversal_gc,
_shenandoah_upgrade_to_full_gc,

==> share/classes/sun/jvm/hotspot/runtime/VMOps.java

It would be good to add 'ShenandoahOperation' to the VMOps enum (though 
it is probably not in sync now).


Thank you,
Jini.

On 12/1/2018 2:30 AM, Roman Kennke wrote:

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.net/~rkennke/shenandoah-upstream/01/

Thanks,
Roman



Hi,

This is the first round of changes for including Shenandoah GC into
mainline.
I divided the review into parts that roughly correspond to the mailing lists
that would normally review it, and I divided it into 'shared' code
changes and
'shenandoah' code changes (actually, mostly additions). The 

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

[Jini George]

Hi Roman,

The SA tests which iterate over the heap (ClhsdbJhisto.java, 
TestHeapDumpFor*.java) fail if ZGC is passed as an option to these also 
(Am planning on fixing these). It might be better to avoid invoking 
those tests for Shenandoah now with an @requires tag.


Thank you,
Jini

On 11/29/2018 3:59 AM, Roman Kennke wrote:

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.net/~rkennke/shenandoah-upstream/01/

Thanks,
Roman



Hi,

This is the first round of changes for including Shenandoah GC into
mainline.
I divided the review into parts that roughly correspond to the mailing lists
that would normally review it, and I divided it into 'shared' code
changes and
'shenandoah' code changes (actually, mostly additions). The intend is to
eventually
push them as single 'combined' changeset, once reviewed.

JEP:
   https://openjdk.java.net/jeps/189
Bug entry:

  https://bugs.openjdk.java.net/browse/JDK-8214259

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

For those who want to see the full change, have a look at the
shenandoah-complete

directory,
it contains the full combined webrev. Alternatively, there is the file
shenandoah-master.patch
,
which is what I intend to commit (and which should be equivalent to the
'shenandoah-complete' webrev).

Sections to review (at this point) are the following:
  *) shenandoah-gc

     - Actual Shenandoah implementation, almost completely residing in
gc/shenandoah

  *) shared-gc

     - This is mostly boilerplate that is common to any GC
     - referenceProcessor.cpp has a little change to make one assert not
fail (next to CMS and G1)
     - taskqueue.hpp has some small adjustments to enable subclassing

  *) shared-serviceability

     - The usual code to support another GC

  *) shared-runtime

     - A number of friends declarations to allow Shenandoah iterators to
hook up with,
   e.g. ClassLoaderData, CodeCache, etc
     - Warning and disabling JFR LeakProfiler
     - fieldDescriptor.hpp added is_stable() accessor, for use in
Shenandoah C2 optimizations
     - Locks initialization in mutexLocker.cpp as usual
     - VM operations defines for Shenandoah's VM ops
     - globalDefinitions.hpp added UINT64_FORMAT_HEX_W for use in
Shenandoah's logging
     - The usual macros in macro.hpp

  *) shared-build

     - Add shenandoah feature, enabled by default, as agreed with
Vladimir K. beforehand
     - Some flags for shenandoah-enabled compilation to get
SUPPORT_BARRIER_ON_PRIMITIVES
   and SUPPORT_NOT_TO_SPACE_INVARIANT which is required for
Shenandoah's barriers
     - --param inline-unit-growth=1000 settings for 2 shenandoah source
files, which is
   useful to get the whole marking loop inlined (observed significant
regression if we
   don't)

  *) shared-tests

     - Test infrastructure to support Shenandoah
     - Shenandoah test groups
     - Exclude Shenandoah in various 

Re: RFR: JDK-8189429: SA: MacOSX: Replace the deprecated PT_ATTACH with PT_ATTACHEXC

[Jini George]

Thank you very much, David. I will do the test-repeat run of the tests 
(after a temp fix to enable OSX runs on Mach5 (JDK-8199700)).


Thanks,
Jini.

On 6/27/2018 12:02 PM, David Holmes wrote:

Hi Jini,

I took a look ... that's about all I can say :)

I know that you and Sharath have worked through this in detail over an 
extended period of time, so I'm okay to add my Reviewed stamp to it.


About the only thing I'd suggest, if not already done, is to do a mach5 
run only on OSX with --test-repeat 10, to try to check that we are okay 
on a range of OSX machines.


Thanks,
David

On 26/06/2018 5:25 PM, Jini George wrote:

Ping - Gentle reminder !

Thanks,
Jini.

On 6/25/2018 3:41 PM, Jini George wrote:
Thank you, Sharath. May I have a Reviewer to take a look at the 
MacosxDebuggerLocal code?


Thanks,
Jini.

On 6/25/2018 1:52 PM, Sharath Ballal wrote:

Hi Jini,

Changes in MacosxDebuggerLocal.m looks good.

Thanks,
Sharath


-Original Message-
From: Jini George
Sent: Sunday, June 24, 2018 11:07 PM
To: Erik Joelsson; serviceability-...@openjdk.java.net; 
build-dev@openjdk.java.net; hotspot-runtime-...@openjdk.java.net
Subject: Re: RFR: JDK-8189429: SA: MacOSX: Replace the deprecated 
PT_ATTACH with PT_ATTACHEXC


Hi Erik,

Thank you very much for looking into this. I have addressed your 
comments. The latest webrev is at:


http://cr.openjdk.java.net/~jgeorge/8189429/webrev.06/

Thank you,
Jini.


On 6/23/2018 3:31 AM, Erik Joelsson wrote:

Hello Jini,

In general this looks pretty good, but it's also breaking some new
ground as it's adding generation of native source in the java gensrc
step. Mixing native code with the java code that the genrcs targets
and gensrc output directories are meant for seems ok for now, but may
cause trouble in the future. I'm going to accept it for now though.

In Gensrc-jdk.hotspot.agent.gmk: Please put the new macosx stuff in
its own section (as delimited by the 80x # lines) and put that whole
thing inside a conditional for macosx. Also please break line 47 (for
recipe lines, indent with tab and 4 additional spaces for 
continuation [1]).


In Lib-jdk.hotspot.agent.gmk: I believe adding extra src should be
enough as that will implicitly add the same directories as header dirs
by default. At least that's the intention.

/Erik

[1] http://openjdk.java.net/groups/build/doc/code-conventions.html

On 2018-06-22 11:11, Jini George wrote:

Hi all,

[Including build-dev also since this includes build related changes].

Requesting reviews for:

https://bugs.openjdk.java.net/browse/JDK-8189429 (SA: MacOSX: Replace
the deprecated PT_ATTACH with PT_ATTACHEXC)

Webrev: http://cr.openjdk.java.net/~jgeorge/8189429/webrev.04/

This is the follow-up solution for the temporary restoration of
PT_ATTACH to fix https://bugs.openjdk.java.net/browse/JDK-8184042
(several serviceability/sa tests timed out on MacOS X), which was
done in Oct 2017. The mails related to this are at:

http://mail.openjdk.java.net/pipermail/serviceability-dev/2017-August
/021702.html


Changes as compared to the patch sent last year
(http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/):

* Addressed the review comments which were provided by Poonam, Dan,
Dmitry.
* A major change as compared to what was done last year is that the
MIG generated files have been included as a part of the JDK build
process.
* The test case which was provided in the patch last year is no
longer required since we have ClhsdbAttach.java testing the same
functionality as a part of the SA testsuite now.
* Other than that, some minor improvements have been done wrt error
handling.

The steps for the proposed solution have been provided in JBS.

Testing: ALL the SA tests pass on MacOSX and the other Mach5 
platforms.


Thanks to Sharath, Robin, Gerard and Dan for looking into the changes
and providing valuable comments.

Thanks,
Jini.

Re: RFR: JDK-8189429: SA: MacOSX: Replace the deprecated PT_ATTACH with PT_ATTACHEXC

[Jini George]

Ping - Gentle reminder !

Thanks,
Jini.

On 6/25/2018 3:41 PM, Jini George wrote:
Thank you, Sharath. May I have a Reviewer to take a look at the 
MacosxDebuggerLocal code?


Thanks,
Jini.

On 6/25/2018 1:52 PM, Sharath Ballal wrote:

Hi Jini,

Changes in MacosxDebuggerLocal.m looks good.

Thanks,
Sharath


-Original Message-
From: Jini George
Sent: Sunday, June 24, 2018 11:07 PM
To: Erik Joelsson; serviceability-...@openjdk.java.net; 
build-dev@openjdk.java.net; hotspot-runtime-...@openjdk.java.net
Subject: Re: RFR: JDK-8189429: SA: MacOSX: Replace the deprecated 
PT_ATTACH with PT_ATTACHEXC


Hi Erik,

Thank you very much for looking into this. I have addressed your 
comments. The latest webrev is at:


http://cr.openjdk.java.net/~jgeorge/8189429/webrev.06/

Thank you,
Jini.


On 6/23/2018 3:31 AM, Erik Joelsson wrote:

Hello Jini,

In general this looks pretty good, but it's also breaking some new
ground as it's adding generation of native source in the java gensrc
step. Mixing native code with the java code that the genrcs targets
and gensrc output directories are meant for seems ok for now, but may
cause trouble in the future. I'm going to accept it for now though.

In Gensrc-jdk.hotspot.agent.gmk: Please put the new macosx stuff in
its own section (as delimited by the 80x # lines) and put that whole
thing inside a conditional for macosx. Also please break line 47 (for
recipe lines, indent with tab and 4 additional spaces for 
continuation [1]).


In Lib-jdk.hotspot.agent.gmk: I believe adding extra src should be
enough as that will implicitly add the same directories as header dirs
by default. At least that's the intention.

/Erik

[1] http://openjdk.java.net/groups/build/doc/code-conventions.html

On 2018-06-22 11:11, Jini George wrote:

Hi all,

[Including build-dev also since this includes build related changes].

Requesting reviews for:

https://bugs.openjdk.java.net/browse/JDK-8189429 (SA: MacOSX: Replace
the deprecated PT_ATTACH with PT_ATTACHEXC)

Webrev: http://cr.openjdk.java.net/~jgeorge/8189429/webrev.04/

This is the follow-up solution for the temporary restoration of
PT_ATTACH to fix https://bugs.openjdk.java.net/browse/JDK-8184042
(several serviceability/sa tests timed out on MacOS X), which was
done in Oct 2017. The mails related to this are at:

http://mail.openjdk.java.net/pipermail/serviceability-dev/2017-August
/021702.html


Changes as compared to the patch sent last year
(http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/):

* Addressed the review comments which were provided by Poonam, Dan,
Dmitry.
* A major change as compared to what was done last year is that the
MIG generated files have been included as a part of the JDK build
process.
* The test case which was provided in the patch last year is no
longer required since we have ClhsdbAttach.java testing the same
functionality as a part of the SA testsuite now.
* Other than that, some minor improvements have been done wrt error
handling.

The steps for the proposed solution have been provided in JBS.

Testing: ALL the SA tests pass on MacOSX and the other Mach5 platforms.

Thanks to Sharath, Robin, Gerard and Dan for looking into the changes
and providing valuable comments.

Thanks,
Jini.

Re: RFR: JDK-8189429: SA: MacOSX: Replace the deprecated PT_ATTACH with PT_ATTACHEXC

[Jini George]

Thank you, Erik !

- Jini.

On 6/25/2018 9:23 PM, Erik Joelsson wrote:

Build changes look good.

/Erik


On 2018-06-24 10:37, Jini George wrote:

Hi Erik,

Thank you very much for looking into this. I have addressed your 
comments. The latest webrev is at:


http://cr.openjdk.java.net/~jgeorge/8189429/webrev.06/

Thank you,
Jini.


On 6/23/2018 3:31 AM, Erik Joelsson wrote:

Hello Jini,

In general this looks pretty good, but it's also breaking some new 
ground as it's adding generation of native source in the java gensrc 
step. Mixing native code with the java code that the genrcs targets 
and gensrc output directories are meant for seems ok for now, but may 
cause trouble in the future. I'm going to accept it for now though.


In Gensrc-jdk.hotspot.agent.gmk: Please put the new macosx stuff in 
its own section (as delimited by the 80x # lines) and put that whole 
thing inside a conditional for macosx. Also please break line 47 (for 
recipe lines, indent with tab and 4 additional spaces for 
continuation [1]).


In Lib-jdk.hotspot.agent.gmk: I believe adding extra src should be 
enough as that will implicitly add the same directories as header 
dirs by default. At least that's the intention.


/Erik

[1] http://openjdk.java.net/groups/build/doc/code-conventions.html

On 2018-06-22 11:11, Jini George wrote:

Hi all,

[Including build-dev also since this includes build related changes].

Requesting reviews for:

https://bugs.openjdk.java.net/browse/JDK-8189429 (SA: MacOSX: 
Replace the deprecated PT_ATTACH with PT_ATTACHEXC)


Webrev: http://cr.openjdk.java.net/~jgeorge/8189429/webrev.04/

This is the follow-up solution for the temporary restoration of 
PT_ATTACH to fix https://bugs.openjdk.java.net/browse/JDK-8184042 
(several serviceability/sa tests timed out on MacOS X), which was 
done in Oct 2017. The mails related to this are at:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2017-August/021702.html 



Changes as compared to the patch sent last year 
(http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/):


* Addressed the review comments which were provided by Poonam, Dan, 
Dmitry.
* A major change as compared to what was done last year is that the 
MIG generated files have been included as a part of the JDK build 
process.
* The test case which was provided in the patch last year is no 
longer required since we have ClhsdbAttach.java testing the same 
functionality as a part of the SA testsuite now.
* Other than that, some minor improvements have been done wrt error 
handling.


The steps for the proposed solution have been provided in JBS.

Testing: ALL the SA tests pass on MacOSX and the other Mach5 platforms.

Thanks to Sharath, Robin, Gerard and Dan for looking into the 
changes and providing valuable comments.


Thanks,
Jini.

Re: RFR: JDK-8189429: SA: MacOSX: Replace the deprecated PT_ATTACH with PT_ATTACHEXC

[Jini George]

Thank you, Sharath. May I have a Reviewer to take a look at the 
MacosxDebuggerLocal code?


Thanks,
Jini.

On 6/25/2018 1:52 PM, Sharath Ballal wrote:

Hi Jini,

Changes in MacosxDebuggerLocal.m looks good.

Thanks,
Sharath


-Original Message-
From: Jini George
Sent: Sunday, June 24, 2018 11:07 PM
To: Erik Joelsson; serviceability-...@openjdk.java.net; 
build-dev@openjdk.java.net; hotspot-runtime-...@openjdk.java.net
Subject: Re: RFR: JDK-8189429: SA: MacOSX: Replace the deprecated PT_ATTACH 
with PT_ATTACHEXC

Hi Erik,

Thank you very much for looking into this. I have addressed your comments. The 
latest webrev is at:

http://cr.openjdk.java.net/~jgeorge/8189429/webrev.06/

Thank you,
Jini.


On 6/23/2018 3:31 AM, Erik Joelsson wrote:

Hello Jini,

In general this looks pretty good, but it's also breaking some new
ground as it's adding generation of native source in the java gensrc
step. Mixing native code with the java code that the genrcs targets
and gensrc output directories are meant for seems ok for now, but may
cause trouble in the future. I'm going to accept it for now though.

In Gensrc-jdk.hotspot.agent.gmk: Please put the new macosx stuff in
its own section (as delimited by the 80x # lines) and put that whole
thing inside a conditional for macosx. Also please break line 47 (for
recipe lines, indent with tab and 4 additional spaces for continuation [1]).

In Lib-jdk.hotspot.agent.gmk: I believe adding extra src should be
enough as that will implicitly add the same directories as header dirs
by default. At least that's the intention.

/Erik

[1] http://openjdk.java.net/groups/build/doc/code-conventions.html

On 2018-06-22 11:11, Jini George wrote:

Hi all,

[Including build-dev also since this includes build related changes].

Requesting reviews for:

https://bugs.openjdk.java.net/browse/JDK-8189429 (SA: MacOSX: Replace
the deprecated PT_ATTACH with PT_ATTACHEXC)

Webrev: http://cr.openjdk.java.net/~jgeorge/8189429/webrev.04/

This is the follow-up solution for the temporary restoration of
PT_ATTACH to fix https://bugs.openjdk.java.net/browse/JDK-8184042
(several serviceability/sa tests timed out on MacOS X), which was
done in Oct 2017. The mails related to this are at:

http://mail.openjdk.java.net/pipermail/serviceability-dev/2017-August
/021702.html


Changes as compared to the patch sent last year
(http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/):

* Addressed the review comments which were provided by Poonam, Dan,
Dmitry.
* A major change as compared to what was done last year is that the
MIG generated files have been included as a part of the JDK build
process.
* The test case which was provided in the patch last year is no
longer required since we have ClhsdbAttach.java testing the same
functionality as a part of the SA testsuite now.
* Other than that, some minor improvements have been done wrt error
handling.

The steps for the proposed solution have been provided in JBS.

Testing: ALL the SA tests pass on MacOSX and the other Mach5 platforms.

Thanks to Sharath, Robin, Gerard and Dan for looking into the changes
and providing valuable comments.

Thanks,
Jini.

Re: RFR: JDK-8189429: SA: MacOSX: Replace the deprecated PT_ATTACH with PT_ATTACHEXC

[Jini George]

Hi Erik,

Thank you very much for looking into this. I have addressed your 
comments. The latest webrev is at:


http://cr.openjdk.java.net/~jgeorge/8189429/webrev.06/

Thank you,
Jini.


On 6/23/2018 3:31 AM, Erik Joelsson wrote:

Hello Jini,

In general this looks pretty good, but it's also breaking some new 
ground as it's adding generation of native source in the java gensrc 
step. Mixing native code with the java code that the genrcs targets and 
gensrc output directories are meant for seems ok for now, but may cause 
trouble in the future. I'm going to accept it for now though.


In Gensrc-jdk.hotspot.agent.gmk: Please put the new macosx stuff in its 
own section (as delimited by the 80x # lines) and put that whole thing 
inside a conditional for macosx. Also please break line 47 (for recipe 
lines, indent with tab and 4 additional spaces for continuation [1]).


In Lib-jdk.hotspot.agent.gmk: I believe adding extra src should be 
enough as that will implicitly add the same directories as header dirs 
by default. At least that's the intention.


/Erik

[1] http://openjdk.java.net/groups/build/doc/code-conventions.html

On 2018-06-22 11:11, Jini George wrote:

Hi all,

[Including build-dev also since this includes build related changes].

Requesting reviews for:

https://bugs.openjdk.java.net/browse/JDK-8189429 (SA: MacOSX: Replace 
the deprecated PT_ATTACH with PT_ATTACHEXC)


Webrev: http://cr.openjdk.java.net/~jgeorge/8189429/webrev.04/

This is the follow-up solution for the temporary restoration of 
PT_ATTACH to fix https://bugs.openjdk.java.net/browse/JDK-8184042 
(several serviceability/sa tests timed out on MacOS X), which was done 
in Oct 2017. The mails related to this are at:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2017-August/021702.html 



Changes as compared to the patch sent last year 
(http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/):


* Addressed the review comments which were provided by Poonam, Dan, 
Dmitry.
* A major change as compared to what was done last year is that the 
MIG generated files have been included as a part of the JDK build 
process.
* The test case which was provided in the patch last year is no longer 
required since we have ClhsdbAttach.java testing the same 
functionality as a part of the SA testsuite now.
* Other than that, some minor improvements have been done wrt error 
handling.


The steps for the proposed solution have been provided in JBS.

Testing: ALL the SA tests pass on MacOSX and the other Mach5 platforms.

Thanks to Sharath, Robin, Gerard and Dan for looking into the changes 
and providing valuable comments.


Thanks,
Jini.

RFR: JDK-8189429: SA: MacOSX: Replace the deprecated PT_ATTACH with PT_ATTACHEXC

[Jini George]

Hi all,

[Including build-dev also since this includes build related changes].

Requesting reviews for:

https://bugs.openjdk.java.net/browse/JDK-8189429 (SA: MacOSX: Replace 
the deprecated PT_ATTACH with PT_ATTACHEXC)


Webrev: http://cr.openjdk.java.net/~jgeorge/8189429/webrev.04/

This is the follow-up solution for the temporary restoration of 
PT_ATTACH to fix https://bugs.openjdk.java.net/browse/JDK-8184042 
(several serviceability/sa tests timed out on MacOS X), which was done 
in Oct 2017. The mails related to this are at:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2017-August/021702.html

Changes as compared to the patch sent last year 
(http://cr.openjdk.java.net/~jgeorge/8184042/webrev.00/):


* Addressed the review comments which were provided by Poonam, Dan, Dmitry.
* A major change as compared to what was done last year is that the MIG 
generated files have been included as a part of the JDK build process.
* The test case which was provided in the patch last year is no longer 
required since we have ClhsdbAttach.java testing the same functionality 
as a part of the SA testsuite now.
* Other than that, some minor improvements have been done wrt error 
handling.


The steps for the proposed solution have been provided in JBS.

Testing: ALL the SA tests pass on MacOSX and the other Mach5 platforms.

Thanks to Sharath, Robin, Gerard and Dan for looking into the changes 
and providing valuable comments.


Thanks,
Jini.

Re: RFR: JDK-8202200: set INCLUDE_SA to false on s390x by default -was : RE: INCLUDE_SA/serviceability agent - support on s390x

[Jini George]

Looks good to me, Matthias.

Thank you,
Jini.

On 4/24/2018 9:39 PM, Baesken, Matthias wrote:

Hi Jini,   the removal  of  the mentioned headers leads to build errors  so we 
better keep it .
Error is when the headers are removed :

/nb/linuxs390x/nightly/jdk/src/hotspot/share/runtime/vmStructs.cpp:100:31: 
fatal error: vmStructs_s390.hpp: No such file or directory
  #include CPU_HEADER(vmStructs)


I  uploaded a webrev for review  :


http://cr.openjdk.java.net/~mbaesken/webrevs/8202200/

bug :

https://bugs.openjdk.java.net/browse/JDK-8202200



Best regards, Matthias



-Original Message-
From: Jini George [mailto:jini.geo...@oracle.com]
Sent: Dienstag, 24. April 2018 08:58
To: Baesken, Matthias <matthias.baes...@sap.com>; 'build-
d...@openjdk.java.net' <build-dev@openjdk.java.net>
Cc: serviceability-...@openjdk.java.net; Schmidt, Lutz
<lutz.schm...@sap.com>
Subject: Re: INCLUDE_SA/serviceability agent - support on s390x

Hi Matthias,

Your change looks good to me. It might make sense to also remove the
following lines from:

src/jdk.hotspot.agent/linux/native/libsaproc/libproc.h

   78 #if defined(s390x)
   79 #include 
   80 #endif

I am not sure if the following files are required either:
src/hotspot/cpu/s390/vmStructs_s390.hpp
src/hotspot/os_cpu/linux_s390/vmStructs_linux_s390.hpp

Thanks,
Jini (Not a Reviewer).


On 4/23/2018 5:31 PM, Baesken, Matthias wrote:

Hello,   as far as I know  the serviceability agent   is not  supported
on  linux s390x .

However  (unlike  on aix where it is not supported as well) ,
   INCLUDE_SA=false    is not set  in the central configure  m4 files .

Should we set it  ( suggested diff below)  ?

Best regards, Matthias

hg diff

diff -r fcd5df7aa235 make/autoconf/jdk-options.m4

--- a/make/autoconf/jdk-options.m4  Wed Apr 18 11:19:32 2018 +0200

+++ b/make/autoconf/jdk-options.m4  Mon Apr 23 13:46:17 2018 +0200

@@ -238,6 +238,9 @@

     if test "x$OPENJDK_TARGET_OS" = xaix ; then

   INCLUDE_SA=false

     fi

+  if test "x$OPENJDK_TARGET_CPU" = xs390x ; then

+    INCLUDE_SA=false

+  fi

     AC_SUBST(INCLUDE_SA)

# Compress jars

Best regards, Matthias

Re: INCLUDE_SA/serviceability agent - support on s390x

[Jini George]

Hi Matthias,

Your change looks good to me. It might make sense to also remove the 
following lines from:


src/jdk.hotspot.agent/linux/native/libsaproc/libproc.h

 78 #if defined(s390x)
 79 #include 
 80 #endif

I am not sure if the following files are required either:
src/hotspot/cpu/s390/vmStructs_s390.hpp
src/hotspot/os_cpu/linux_s390/vmStructs_linux_s390.hpp

Thanks,
Jini (Not a Reviewer).


On 4/23/2018 5:31 PM, Baesken, Matthias wrote:
Hello,   as far as I know  the serviceability agent   is not  supported 
on  linux s390x .


However  (unlike  on aix where it is not supported as well) ,  
  INCLUDE_SA=false    is not set  in the central configure  m4 files .


Should we set it  ( suggested diff below)  ?

Best regards, Matthias

hg diff

diff -r fcd5df7aa235 make/autoconf/jdk-options.m4

--- a/make/autoconf/jdk-options.m4  Wed Apr 18 11:19:32 2018 +0200

+++ b/make/autoconf/jdk-options.m4  Mon Apr 23 13:46:17 2018 +0200

@@ -238,6 +238,9 @@

    if test "x$OPENJDK_TARGET_OS" = xaix ; then

  INCLUDE_SA=false

    fi

+  if test "x$OPENJDK_TARGET_CPU" = xs390x ; then

+    INCLUDE_SA=false

+  fi

    AC_SUBST(INCLUDE_SA)

# Compress jars

Best regards, Matthias