Re: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread David Holmes

Hi Amit,

Just a couple of style nits:

! iterations++;
!
! if (10 == iterations) {

=>

  if (++iterations == 10) {
--

!iterations=0;

=>
 iterations = 0;

Thanks,
David

On 23/02/2017 4:58 PM, Amit Sapre wrote:

Hello,

http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.02/ has the 
updated changes.

I ran this test on my VM and roughly takes 250-300 ms to get all the listener 
count.

Thanks,
Amit


-Original Message-
From: Amit Sapre
Sent: Thursday, February 23, 2017 11:55 AM
To: David Holmes; serviceability-dev@openjdk.java.net; Harsha Wardhana
B
Subject: RE: RFR : JDK-7132577 -
javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

Hello,

In a messy run of this test case,
the number of prints for the counter values will be 240 (120 seconds is
jtreg timeout)

Will try to optimize this. Please hold on for a new webrev.

Thanks,
Amit



-Original Message-
From: Amit Sapre
Sent: Thursday, February 23, 2017 11:48 AM
To: David Holmes; serviceability-dev@openjdk.java.net; Harsha

Wardhana

B
Subject: RE: RFR : JDK-7132577 -
javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

Hello,

Thanks David & Harsha for your inputs.
Here is the new webrev :
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.01/

Thanks,
Amit


-Original Message-
From: David Holmes
Sent: Thursday, February 23, 2017 9:24 AM
To: serviceability-dev@openjdk.java.net; Sapre Amit
Cc: Harsha Wardhana B
Subject: Re: RFR : JDK-7132577 -
javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

Hi Amit,

On 23/02/2017 12:18 AM, Harsha Wardhana B wrote:

Hi Amit,

There is no need to wait in a loop to check we have not received

any

notifications. Without starting the monitors, the listener count
will be zero.

The first part of diff L116-L121 could be left as is.


Agreed.


By relying on Jtreg timeout for receiving notifications, we will
be unable to print the number of listeners emitted by each

Monitor.

But

I

guess there is no way to intercept a Jtreg timeout and print out

those

values.


You could print out the values every 1 second, or 5 or 10, ... at
least that way we can see what the values are when we timeout, and
also if they have been changing.

Thanks,
David


Regards

Harsha


On Wednesday 22 February 2017 03:29 PM, Amit Sapre wrote:


Hello,



Please review this test bug fix which eliminates test case's own
timeout mechanism to default jtreg timeout.



Bug ID : https://bugs.openjdk.java.net/browse/JDK-7132577

Webrev :
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-

7132577/webrev.00/





Thanks,

Amit





RE: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread Amit Sapre
Hello,

http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.02/ has the 
updated changes.

I ran this test on my VM and roughly takes 250-300 ms to get all the listener 
count.

Thanks,
Amit

> -Original Message-
> From: Amit Sapre
> Sent: Thursday, February 23, 2017 11:55 AM
> To: David Holmes; serviceability-dev@openjdk.java.net; Harsha Wardhana
> B
> Subject: RE: RFR : JDK-7132577 -
> javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22
> 
> Hello,
> 
> In a messy run of this test case,
> the number of prints for the counter values will be 240 (120 seconds is
> jtreg timeout)
> 
> Will try to optimize this. Please hold on for a new webrev.
> 
> Thanks,
> Amit
> 
> 
> > -Original Message-
> > From: Amit Sapre
> > Sent: Thursday, February 23, 2017 11:48 AM
> > To: David Holmes; serviceability-dev@openjdk.java.net; Harsha
> Wardhana
> > B
> > Subject: RE: RFR : JDK-7132577 -
> > javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22
> >
> > Hello,
> >
> > Thanks David & Harsha for your inputs.
> > Here is the new webrev :
> > http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.01/
> >
> > Thanks,
> > Amit
> >
> > > -Original Message-
> > > From: David Holmes
> > > Sent: Thursday, February 23, 2017 9:24 AM
> > > To: serviceability-dev@openjdk.java.net; Sapre Amit
> > > Cc: Harsha Wardhana B
> > > Subject: Re: RFR : JDK-7132577 -
> > > javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22
> > >
> > > Hi Amit,
> > >
> > > On 23/02/2017 12:18 AM, Harsha Wardhana B wrote:
> > > > Hi Amit,
> > > >
> > > > There is no need to wait in a loop to check we have not received
> > any
> > > > notifications. Without starting the monitors, the listener count
> > > > will be zero.
> > > >
> > > > The first part of diff L116-L121 could be left as is.
> > >
> > > Agreed.
> > >
> > > > By relying on Jtreg timeout for receiving notifications, we will
> > > > be unable to print the number of listeners emitted by each
> Monitor.
> > But
> > > I
> > > > guess there is no way to intercept a Jtreg timeout and print out
> > > those
> > > > values.
> > >
> > > You could print out the values every 1 second, or 5 or 10, ... at
> > > least that way we can see what the values are when we timeout, and
> > > also if they have been changing.
> > >
> > > Thanks,
> > > David
> > >
> > > > Regards
> > > >
> > > > Harsha
> > > >
> > > >
> > > > On Wednesday 22 February 2017 03:29 PM, Amit Sapre wrote:
> > > >>
> > > >> Hello,
> > > >>
> > > >>
> > > >>
> > > >> Please review this test bug fix which eliminates test case's own
> > > >> timeout mechanism to default jtreg timeout.
> > > >>
> > > >>
> > > >>
> > > >> Bug ID : https://bugs.openjdk.java.net/browse/JDK-7132577
> > > >>
> > > >> Webrev :
> > > >> http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-
> > > 7132577/webrev.00/
> > > >>  > > 7132577/webrev.
> > > >> 00/>
> > > >>
> > > >>
> > > >>
> > > >> Thanks,
> > > >>
> > > >> Amit
> > > >>
> > > >


Re: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread David Holmes

Hi Amit,

On 23/02/2017 4:18 PM, Amit Sapre wrote:

Hello,

Thanks David & Harsha for your inputs.
Here is the new webrev : 
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.01/


You didn't leave L116 - L121 as-is, you deleted them. Isn't checking 
they are all 0 a useful sanity check?


Moving the printing into listenersAreAll is not ideal as the only way to 
control the output rate is by changing the sleep time in the loop. But 
we want the test to be responsive (no big sleeps) and also don't want 
excessive output. The 500ms sleep might be a reasonable compromise but I 
can't tell that - how long does the test normally take to run and how 
much output is generated?


Thanks,
David


Thanks,
Amit


-Original Message-
From: David Holmes
Sent: Thursday, February 23, 2017 9:24 AM
To: serviceability-dev@openjdk.java.net; Sapre Amit
Cc: Harsha Wardhana B
Subject: Re: RFR : JDK-7132577 -
javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

Hi Amit,

On 23/02/2017 12:18 AM, Harsha Wardhana B wrote:

Hi Amit,

There is no need to wait in a loop to check we have not received any
notifications. Without starting the monitors, the listener count will
be zero.

The first part of diff L116-L121 could be left as is.


Agreed.


By relying on Jtreg timeout for receiving notifications, we will be
unable to print the number of listeners emitted by each Monitor. But

I

guess there is no way to intercept a Jtreg timeout and print out

those

values.


You could print out the values every 1 second, or 5 or 10, ... at least
that way we can see what the values are when we timeout, and also if
they have been changing.

Thanks,
David


Regards

Harsha


On Wednesday 22 February 2017 03:29 PM, Amit Sapre wrote:


Hello,



Please review this test bug fix which eliminates test case's own
timeout mechanism to default jtreg timeout.



Bug ID : https://bugs.openjdk.java.net/browse/JDK-7132577

Webrev :
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-

7132577/webrev.00/





Thanks,

Amit





RE: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread Amit Sapre
Hello,

Thanks David & Harsha for your inputs.
Here is the new webrev : 
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.01/

Thanks,
Amit

> -Original Message-
> From: David Holmes
> Sent: Thursday, February 23, 2017 9:24 AM
> To: serviceability-dev@openjdk.java.net; Sapre Amit
> Cc: Harsha Wardhana B
> Subject: Re: RFR : JDK-7132577 -
> javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22
> 
> Hi Amit,
> 
> On 23/02/2017 12:18 AM, Harsha Wardhana B wrote:
> > Hi Amit,
> >
> > There is no need to wait in a loop to check we have not received any
> > notifications. Without starting the monitors, the listener count will
> > be zero.
> >
> > The first part of diff L116-L121 could be left as is.
> 
> Agreed.
> 
> > By relying on Jtreg timeout for receiving notifications, we will be
> > unable to print the number of listeners emitted by each Monitor. But
> I
> > guess there is no way to intercept a Jtreg timeout and print out
> those
> > values.
> 
> You could print out the values every 1 second, or 5 or 10, ... at least
> that way we can see what the values are when we timeout, and also if
> they have been changing.
> 
> Thanks,
> David
> 
> > Regards
> >
> > Harsha
> >
> >
> > On Wednesday 22 February 2017 03:29 PM, Amit Sapre wrote:
> >>
> >> Hello,
> >>
> >>
> >>
> >> Please review this test bug fix which eliminates test case's own
> >> timeout mechanism to default jtreg timeout.
> >>
> >>
> >>
> >> Bug ID : https://bugs.openjdk.java.net/browse/JDK-7132577
> >>
> >> Webrev :
> >> http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-
> 7132577/webrev.00/
> >>  7132577/webrev.
> >> 00/>
> >>
> >>
> >>
> >> Thanks,
> >>
> >> Amit
> >>
> >


Re: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread David Holmes

Hi Amit,

On 23/02/2017 12:18 AM, Harsha Wardhana B wrote:

Hi Amit,

There is no need to wait in a loop to check we have not received any
notifications. Without starting the monitors, the listener count will be
zero.

The first part of diff L116-L121 could be left as is.


Agreed.


By relying on Jtreg timeout for receiving notifications, we will be
unable to print the number of listeners emitted by each Monitor. But I
guess there is no way to intercept a Jtreg timeout and print out those
values.


You could print out the values every 1 second, or 5 or 10, ... at least 
that way we can see what the values are when we timeout, and also if 
they have been changing.


Thanks,
David


Regards

Harsha


On Wednesday 22 February 2017 03:29 PM, Amit Sapre wrote:


Hello,



Please review this test bug fix which eliminates test case’s own
 timeout mechanism to default jtreg timeout.



Bug ID : https://bugs.openjdk.java.net/browse/JDK-7132577

Webrev :
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.00/




Thanks,

Amit





Re: RFR: JDK-8173130 - SubjectDelegation2Test.java and SubjectDelegation3Test.java failing on solaris

2017-02-22 Thread Harsha Wardhana B

Thanks for the review Daniel.

-Harsha


On Wednesday 22 February 2017 08:21 PM, Daniel Fuchs wrote:

Hi Harsha,

OK - makes sense to me.
Maybe just wait for tomorrow to see if you get any other comments
form other reviewers...

best regards,

-- daniel

On 22/02/17 14:38, Harsha Wardhana B wrote:

Hi Daniel,

("java.lang.RuntimePermission" "accessClassInPackage.sun.security.krb5")
was missing.

I gave all the RuntimePermissions since we don't want test cases to fail
in future.

Granting java.lang.RuntimePermission "*" does not affect the test cases
in anyway as test cases rely on delegation of specific permissions.

Regards
Harsha

On Wednesday 22 February 2017 07:43 PM, Daniel Fuchs wrote:

Hi Harsha,

Looks good - but do you really need java.lang.RuntimePermission "*" ?

What permission was missing?

best regards,

-- daniel

On 22/02/17 13:35, Harsha Wardhana B wrote:

Hi All,

Please review the below test fix for

Issue: JDK-8173130  - SubjectDelegation2Test.java
and SubjectDelegation3Test.java failing on solaris

webrev : http://cr.openjdk.java.net/~hb/8173130/webrev.00/

The fix does the following.

1. Removes explicit platform check

2. Fixes JMXServiceURL

3. Adds required RuntimePermissions for test to execute.

Regards

Harsha











Re: RFR: JDK-8173130 - SubjectDelegation2Test.java and SubjectDelegation3Test.java failing on solaris

2017-02-22 Thread Daniel Fuchs

Hi Harsha,

OK - makes sense to me.
Maybe just wait for tomorrow to see if you get any other comments
form other reviewers...

best regards,

-- daniel

On 22/02/17 14:38, Harsha Wardhana B wrote:

Hi Daniel,

("java.lang.RuntimePermission" "accessClassInPackage.sun.security.krb5")
was missing.

I gave all the RuntimePermissions since we don't want test cases to fail
in future.

Granting java.lang.RuntimePermission "*" does not affect the test cases
in anyway as test cases rely on delegation of specific permissions.

Regards
Harsha

On Wednesday 22 February 2017 07:43 PM, Daniel Fuchs wrote:

Hi Harsha,

Looks good - but do you really need java.lang.RuntimePermission "*" ?

What permission was missing?

best regards,

-- daniel

On 22/02/17 13:35, Harsha Wardhana B wrote:

Hi All,

Please review the below test fix for

Issue: JDK-8173130  - SubjectDelegation2Test.java
and SubjectDelegation3Test.java failing on solaris

webrev : http://cr.openjdk.java.net/~hb/8173130/webrev.00/

The fix does the following.

1. Removes explicit platform check

2. Fixes JMXServiceURL

3. Adds required RuntimePermissions for test to execute.

Regards

Harsha









Re: RFR: JDK-8173130 - SubjectDelegation2Test.java and SubjectDelegation3Test.java failing on solaris

2017-02-22 Thread Harsha Wardhana B

Hi Daniel,

("java.lang.RuntimePermission" "accessClassInPackage.sun.security.krb5") 
was missing.


I gave all the RuntimePermissions since we don't want test cases to fail 
in future.


Granting java.lang.RuntimePermission "*" does not affect the test cases 
in anyway as test cases rely on delegation of specific permissions.


Regards
Harsha

On Wednesday 22 February 2017 07:43 PM, Daniel Fuchs wrote:

Hi Harsha,

Looks good - but do you really need java.lang.RuntimePermission "*" ?

What permission was missing?

best regards,

-- daniel

On 22/02/17 13:35, Harsha Wardhana B wrote:

Hi All,

Please review the below test fix for

Issue: JDK-8173130  - SubjectDelegation2Test.java
and SubjectDelegation3Test.java failing on solaris

webrev : http://cr.openjdk.java.net/~hb/8173130/webrev.00/

The fix does the following.

1. Removes explicit platform check

2. Fixes JMXServiceURL

3. Adds required RuntimePermissions for test to execute.

Regards

Harsha







Re: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread Harsha Wardhana B

Hi Amit,

There is no need to wait in a loop to check we have not received any 
notifications. Without starting the monitors, the listener count will be 
zero.


The first part of diff L116-L121 could be left as is.

By relying on Jtreg timeout for receiving notifications, we will be 
unable to print the number of listeners emitted by each Monitor. But I 
guess there is no way to intercept a Jtreg timeout and print out those 
values.


Regards

Harsha


On Wednesday 22 February 2017 03:29 PM, Amit Sapre wrote:


Hello,

Please review this test bug fix which eliminates test case’s own 
 timeout mechanism to default jtreg timeout.


Bug ID : https://bugs.openjdk.java.net/browse/JDK-7132577

Webrev : 
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.00/ 



Thanks,

Amit





Re: RFR: JDK-8173130 - SubjectDelegation2Test.java and SubjectDelegation3Test.java failing on solaris

2017-02-22 Thread Daniel Fuchs

Hi Harsha,

Looks good - but do you really need java.lang.RuntimePermission "*" ?

What permission was missing?

best regards,

-- daniel

On 22/02/17 13:35, Harsha Wardhana B wrote:

Hi All,

Please review the below test fix for

Issue: JDK-8173130  - SubjectDelegation2Test.java
and SubjectDelegation3Test.java failing on solaris

webrev : http://cr.openjdk.java.net/~hb/8173130/webrev.00/

The fix does the following.

1. Removes explicit platform check

2. Fixes JMXServiceURL

3. Adds required RuntimePermissions for test to execute.

Regards

Harsha





RFR: JDK-8173130 - SubjectDelegation2Test.java and SubjectDelegation3Test.java failing on solaris

2017-02-22 Thread Harsha Wardhana B

Hi All,

Please review the below test fix for

Issue: JDK-8173130  - SubjectDelegation2Test.java 
and SubjectDelegation3Test.java failing on solaris


webrev : http://cr.openjdk.java.net/~hb/8173130/webrev.00/

The fix does the following.

1. Removes explicit platform check

2. Fixes JMXServiceURL

3. Adds required RuntimePermissions for test to execute.

Regards

Harsha



Re: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread Erik Gahlin

Looks good.

Erik


Hello,

Please review this test bug fix which eliminates test case’s own 
 timeout mechanism to default jtreg timeout.


Bug ID : https://bugs.openjdk.java.net/browse/JDK-7132577

Webrev : 
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.00/ 



Thanks,

Amit





RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread Amit Sapre
Hello,

 

Please review this test bug fix which eliminates test case's own  timeout 
mechanism to default jtreg timeout.

 

Bug ID : https://bugs.openjdk.java.net/browse/JDK-7132577 

Webrev : http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.00/ 

 

Thanks,

Amit


Re: RFR: JDK-8162504: [SA testbug] TestInstanceKlassSize.java and TestInstanceKlassSizeForInterface.java fail on Mac OS

2017-02-22 Thread Jini George

Thank you, Serguei.

- jini.


On 2/22/2017 12:58 PM, serguei.spit...@oracle.com wrote:

Hi Jini,

It looks good.

Thanks,
Serguei


On 2/21/17 07:48, Jini George wrote:

Requesting reviews for the following SA testbug:

https://bugs.openjdk.java.net/browse/JDK-8162504
Webrev: http://cr.openjdk.java.net/~jgeorge/8162504/webrev.00/

Thank you,
Jini.