RFR (S): 8193258: Better usage of JDWP HEADER SIZE

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR (S): 8193258: Better usage of JDWP HEADER SIZE

Langer, Christoph

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…

 

Best regards

Christoph

 

Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

serguei.spitsyn@oracle.com
Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 


Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

Chris Plummer
+1

Thanks for cleaning this up.

Chris

On 12/8/17 12:12 PM, [hidden email] wrote:
Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 



Reply | Threaded
Open this post in threaded view
|

RE: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

Langer, Christoph
In reply to this post by serguei.spitsyn@oracle.com

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [mailto:[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph <[hidden email]>; [hidden email]; Chris Plummer <[hidden email]>
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 

Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

serguei.spitsyn@oracle.com
Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 


Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

Chris Plummer
I think I ran everything svc related. I did the following

open/test/hotspot/jtreg/:hotspot_serviceability
open/test/jdk/:jdk_svc

Plus the closed equivalents and all our other closed svc test lists.

Chris

On 12/8/17 9:26 PM, [hidden email] wrote:
Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 



Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

serguei.spitsyn@oracle.com
Hi Chris,

Thank you, Chris for doing this.
In general, it is still a good idea for Christoph to always run the com/sun/jdi tests locally.
The hotspot/jtreg/serviceability/jdwp tests do not provide a complete coverage.

Thanks,
Serguei


On 12/8/17 21:46, Chris Plummer wrote:
I think I ran everything svc related. I did the following

open/test/hotspot/jtreg/:hotspot_serviceability
open/test/jdk/:jdk_svc

Plus the closed equivalents and all our other closed svc test lists.

Chris

On 12/8/17 9:26 PM, [hidden email] wrote:
Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 




Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

Chris Plummer
Yes, those are covered by :jdk_svc, but he should also run them first.

Chris

On 12/8/17 9:56 PM, [hidden email] wrote:
Hi Chris,

Thank you, Chris for doing this.
In general, it is still a good idea for Christoph to always run the com/sun/jdi tests locally.
The hotspot/jtreg/serviceability/jdwp tests do not provide a complete coverage.

Thanks,
Serguei


On 12/8/17 21:46, Chris Plummer wrote:
I think I ran everything svc related. I did the following

open/test/hotspot/jtreg/:hotspot_serviceability
open/test/jdk/:jdk_svc

Plus the closed equivalents and all our other closed svc test lists.

Chris

On 12/8/17 9:26 PM, [hidden email] wrote:
Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 





Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

serguei.spitsyn@oracle.com
Right.

Thanks,
Serguei


On 12/8/17 22:14, Chris Plummer wrote:
Yes, those are covered by :jdk_svc, but he should also run them first.

Chris

On 12/8/17 9:56 PM, [hidden email] wrote:
Hi Chris,

Thank you, Chris for doing this.
In general, it is still a good idea for Christoph to always run the com/sun/jdi tests locally.
The hotspot/jtreg/serviceability/jdwp tests do not provide a complete coverage.

Thanks,
Serguei


On 12/8/17 21:46, Chris Plummer wrote:
I think I ran everything svc related. I did the following

open/test/hotspot/jtreg/:hotspot_serviceability
open/test/jdk/:jdk_svc

Plus the closed equivalents and all our other closed svc test lists.

Chris

On 12/8/17 9:26 PM, [hidden email] wrote:
Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 






Reply | Threaded
Open this post in threaded view
|

RE: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

Langer, Christoph
In reply to this post by serguei.spitsyn@oracle.com

Hi Serguei,

 

I wasn’t aware that the jdk/com/sun/jdi tests would test libjdwp, I just found hotspot/jtreg/serviceability/jdwp when searching for jdwp in the tests. But thanks to your hints I know better now. In fact I ran the jdi tests when testing my change for 8193183.

 

I made a new webrev which includes the removal of inStream_endOfInput: http://cr.openjdk.java.net/~clanger/webrevs/8193258.1/. Can you please approve this.

 

This time I ran the jdi tests without issues and also did builds on Windows, linux x86, AIX, Solaris and Mac. J

 

Best regards

Christoph

 

 

 

From: [hidden email] [mailto:[hidden email]]
Sent: Samstag, 9. Dezember 2017 06:27
To: Langer, Christoph <[hidden email]>; [hidden email]; Chris Plummer <[hidden email]>
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 

 

Reply | Threaded
Open this post in threaded view
|

RE: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

Langer, Christoph

Hi Serguei, Chris,

 

please look at this webrev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.2/

 

I spotted a few other locations in libdt_shmem where JDWP HEADER SIZE should be used. Builds and tests running…

 

Best regards

Christoph

 

From: Langer, Christoph
Sent: Sonntag, 10. Dezember 2017 10:52
To: '[hidden email]' <[hidden email]>; [hidden email]
Cc: Chris Plummer <[hidden email]>
Subject: RE: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Serguei,

 

I wasn’t aware that the jdk/com/sun/jdi tests would test libjdwp, I just found hotspot/jtreg/serviceability/jdwp when searching for jdwp in the tests. But thanks to your hints I know better now. In fact I ran the jdi tests when testing my change for 8193183.

 

I made a new webrev which includes the removal of inStream_endOfInput: http://cr.openjdk.java.net/~clanger/webrevs/8193258.1/. Can you please approve this.

 

This time I ran the jdi tests without issues and also did builds on Windows, linux x86, AIX, Solaris and Mac. J

 

Best regards

Christoph

 

 

 

From: [hidden email] [[hidden email]]
Sent: Samstag, 9. Dezember 2017 06:27
To: Langer, Christoph <[hidden email]>; [hidden email]; Chris Plummer <[hidden email]>
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 

 

Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

Chris Plummer
Looks good.

thanks,

Chris

On 12/11/17 7:43 AM, Langer, Christoph wrote:

Hi Serguei, Chris,

 

please look at this webrev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.2/

 

I spotted a few other locations in libdt_shmem where JDWP HEADER SIZE should be used. Builds and tests running…

 

Best regards

Christoph

 

From: Langer, Christoph
Sent: Sonntag, 10. Dezember 2017 10:52
To: '[hidden email]' [hidden email]; [hidden email]
Cc: Chris Plummer [hidden email]
Subject: RE: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Serguei,

 

I wasn’t aware that the jdk/com/sun/jdi tests would test libjdwp, I just found hotspot/jtreg/serviceability/jdwp when searching for jdwp in the tests. But thanks to your hints I know better now. In fact I ran the jdi tests when testing my change for 8193183.

 

I made a new webrev which includes the removal of inStream_endOfInput: http://cr.openjdk.java.net/~clanger/webrevs/8193258.1/. Can you please approve this.

 

This time I ran the jdi tests without issues and also did builds on Windows, linux x86, AIX, Solaris and Mac. J

 

Best regards

Christoph

 

 

 

From: [hidden email] [[hidden email]]
Sent: Samstag, 9. Dezember 2017 06:27
To: Langer, Christoph <[hidden email]>; [hidden email]; Chris Plummer <[hidden email]>
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 

 


Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

serguei.spitsyn@oracle.com
In reply to this post by Langer, Christoph
Hi Christoph,

The fix looks good to me.
I'll submit a mach5 job to make sure nothing is broken.

Thanks,
Serguei



On 12/11/17 07:43, Langer, Christoph wrote:

Hi Serguei, Chris,

 

please look at this webrev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.2/

 

I spotted a few other locations in libdt_shmem where JDWP HEADER SIZE should be used. Builds and tests running…

 

Best regards

Christoph

 

From: Langer, Christoph
Sent: Sonntag, 10. Dezember 2017 10:52
To: '[hidden email]' [hidden email]; [hidden email]
Cc: Chris Plummer [hidden email]
Subject: RE: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Serguei,

 

I wasn’t aware that the jdk/com/sun/jdi tests would test libjdwp, I just found hotspot/jtreg/serviceability/jdwp when searching for jdwp in the tests. But thanks to your hints I know better now. In fact I ran the jdi tests when testing my change for 8193183.

 

I made a new webrev which includes the removal of inStream_endOfInput: http://cr.openjdk.java.net/~clanger/webrevs/8193258.1/. Can you please approve this.

 

This time I ran the jdi tests without issues and also did builds on Windows, linux x86, AIX, Solaris and Mac. J

 

Best regards

Christoph

 

 

 

From: [hidden email] [[hidden email]]
Sent: Samstag, 9. Dezember 2017 06:27
To: Langer, Christoph <[hidden email]>; [hidden email]; Chris Plummer <[hidden email]>
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 

 


Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

serguei.spitsyn@oracle.com
Hi Christoph,

The testing looks good.
These are the results:
  http://java.se.oracle.com:10065/mdash/jobs/sspitsyn-clanger-JDK-8193258-20171212-0054-7258

The job includes hs-tier's from 1-5.
There are some failures there but nothing related to the JDI or JDWP tests.

Thanks,
Serguei


On 12/11/17 11:10, [hidden email] wrote:
Hi Christoph,

The fix looks good to me.
I'll submit a mach5 job to make sure nothing is broken.

Thanks,
Serguei



On 12/11/17 07:43, Langer, Christoph wrote:

Hi Serguei, Chris,

 

please look at this webrev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.2/

 

I spotted a few other locations in libdt_shmem where JDWP HEADER SIZE should be used. Builds and tests running…

 

Best regards

Christoph

 

From: Langer, Christoph
Sent: Sonntag, 10. Dezember 2017 10:52
To: '[hidden email]' [hidden email]; [hidden email]
Cc: Chris Plummer [hidden email]
Subject: RE: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Serguei,

 

I wasn’t aware that the jdk/com/sun/jdi tests would test libjdwp, I just found hotspot/jtreg/serviceability/jdwp when searching for jdwp in the tests. But thanks to your hints I know better now. In fact I ran the jdi tests when testing my change for 8193183.

 

I made a new webrev which includes the removal of inStream_endOfInput: http://cr.openjdk.java.net/~clanger/webrevs/8193258.1/. Can you please approve this.

 

This time I ran the jdi tests without issues and also did builds on Windows, linux x86, AIX, Solaris and Mac. J

 

Best regards

Christoph

 

 

 

From: [hidden email] [[hidden email]]
Sent: Samstag, 9. Dezember 2017 06:27
To: Langer, Christoph <[hidden email]>; [hidden email]; Chris Plummer <[hidden email]>
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 

 



Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

Chris Plummer
That's an internal link. Christoph won't be able to see them.

Chris

On 12/11/17 9:50 PM, [hidden email] wrote:
Hi Christoph,

The testing looks good.
These are the results:
  http://java.se.oracle.com:10065/mdash/jobs/sspitsyn-clanger-JDK-8193258-20171212-0054-7258

The job includes hs-tier's from 1-5.
There are some failures there but nothing related to the JDI or JDWP tests.

Thanks,
Serguei


On 12/11/17 11:10, [hidden email] wrote:
Hi Christoph,

The fix looks good to me.
I'll submit a mach5 job to make sure nothing is broken.

Thanks,
Serguei



On 12/11/17 07:43, Langer, Christoph wrote:

Hi Serguei, Chris,

 

please look at this webrev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.2/

 

I spotted a few other locations in libdt_shmem where JDWP HEADER SIZE should be used. Builds and tests running…

 

Best regards

Christoph

 

From: Langer, Christoph
Sent: Sonntag, 10. Dezember 2017 10:52
To: '[hidden email]' [hidden email]; [hidden email]
Cc: Chris Plummer [hidden email]
Subject: RE: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Serguei,

 

I wasn’t aware that the jdk/com/sun/jdi tests would test libjdwp, I just found hotspot/jtreg/serviceability/jdwp when searching for jdwp in the tests. But thanks to your hints I know better now. In fact I ran the jdi tests when testing my change for 8193183.

 

I made a new webrev which includes the removal of inStream_endOfInput: http://cr.openjdk.java.net/~clanger/webrevs/8193258.1/. Can you please approve this.

 

This time I ran the jdi tests without issues and also did builds on Windows, linux x86, AIX, Solaris and Mac. J

 

Best regards

Christoph

 

 

 

From: [hidden email] [[hidden email]]
Sent: Samstag, 9. Dezember 2017 06:27
To: Langer, Christoph <[hidden email]>; [hidden email]; Chris Plummer <[hidden email]>
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 

 




Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

serguei.spitsyn@oracle.com
Yes.
That was stupid. :(

Thanks, Chris!
Serguei


On 12/11/17 22:24, Chris Plummer wrote:
That's an internal link. Christoph won't be able to see them.

Chris

On 12/11/17 9:50 PM, [hidden email] wrote:
Hi Christoph,

The testing looks good.
These are the results:
  http://java.se.oracle.com:10065/mdash/jobs/sspitsyn-clanger-JDK-8193258-20171212-0054-7258

The job includes hs-tier's from 1-5.
There are some failures there but nothing related to the JDI or JDWP tests.

Thanks,
Serguei


On 12/11/17 11:10, [hidden email] wrote:
Hi Christoph,

The fix looks good to me.
I'll submit a mach5 job to make sure nothing is broken.

Thanks,
Serguei



On 12/11/17 07:43, Langer, Christoph wrote:

Hi Serguei, Chris,

 

please look at this webrev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.2/

 

I spotted a few other locations in libdt_shmem where JDWP HEADER SIZE should be used. Builds and tests running…

 

Best regards

Christoph

 

From: Langer, Christoph
Sent: Sonntag, 10. Dezember 2017 10:52
To: '[hidden email]' [hidden email]; [hidden email]
Cc: Chris Plummer [hidden email]
Subject: RE: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Serguei,

 

I wasn’t aware that the jdk/com/sun/jdi tests would test libjdwp, I just found hotspot/jtreg/serviceability/jdwp when searching for jdwp in the tests. But thanks to your hints I know better now. In fact I ran the jdi tests when testing my change for 8193183.

 

I made a new webrev which includes the removal of inStream_endOfInput: http://cr.openjdk.java.net/~clanger/webrevs/8193258.1/. Can you please approve this.

 

This time I ran the jdi tests without issues and also did builds on Windows, linux x86, AIX, Solaris and Mac. J

 

Best regards

Christoph

 

 

 

From: [hidden email] [[hidden email]]
Sent: Samstag, 9. Dezember 2017 06:27
To: Langer, Christoph <[hidden email]>; [hidden email]; Chris Plummer <[hidden email]>
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph

 

 

 





Reply | Threaded
Open this post in threaded view
|

RE: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

Langer, Christoph

Hi,

 

no problem. I already thought that I would not be able to see this.

 

I pushed it: http://hg.openjdk.java.net/jdk/jdk/rev/61e60548c0cf

 

Thanks for all your help in reviewing/testing this.

 

Best regards

Christoph

 

From: [hidden email] [mailto:[hidden email]]
Sent: Dienstag, 12. Dezember 2017 07:35
To: Chris Plummer <[hidden email]>; Langer, Christoph <[hidden email]>; [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Yes.
That was stupid. :(

Thanks, Chris!
Serguei


On 12/11/17 22:24, Chris Plummer wrote:

That's an internal link. Christoph won't be able to see them.

Chris

On 12/11/17 9:50 PM, [hidden email] wrote:

Hi Christoph,

The testing looks good.
These are the results:
  http://java.se.oracle.com:10065/mdash/jobs/sspitsyn-clanger-JDK-8193258-20171212-0054-7258

The job includes hs-tier's from 1-5.
There are some failures there but nothing related to the JDI or JDWP tests.

Thanks,
Serguei


On 12/11/17 11:10, [hidden email] wrote:

Hi Christoph,

The fix looks good to me.
I'll submit a mach5 job to make sure nothing is broken.

Thanks,
Serguei



On 12/11/17 07:43, Langer, Christoph wrote:

Hi Serguei, Chris,

 

please look at this webrev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.2/

 

I spotted a few other locations in libdt_shmem where JDWP HEADER SIZE should be used. Builds and tests running…

 

Best regards

Christoph

 

From: Langer, Christoph
Sent: Sonntag, 10. Dezember 2017 10:52
To: '[hidden email]' [hidden email]; [hidden email]
Cc: Chris Plummer [hidden email]
Subject: RE: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Serguei,

 

I wasn’t aware that the jdk/com/sun/jdi tests would test libjdwp, I just found hotspot/jtreg/serviceability/jdwp when searching for jdwp in the tests. But thanks to your hints I know better now. In fact I ran the jdi tests when testing my change for 8193183.

 

I made a new webrev which includes the removal of inStream_endOfInput: http://cr.openjdk.java.net/~clanger/webrevs/8193258.1/. Can you please approve this.

 

This time I ran the jdi tests without issues and also did builds on Windows, linux x86, AIX, Solaris and Mac. J

 

Best regards

Christoph

 

 

 

From: [hidden email] [[hidden email]]
Sent: Samstag, 9. Dezember 2017 06:27
To: Langer, Christoph <[hidden email]>; [hidden email]; Chris Plummer <[hidden email]>
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

You need to run at least the jdk/com/sun/jdi tests.

Thanks,
Serguei


On 12/8/17 13:07, Langer, Christoph wrote:

Hi Serguei,

 

I did only run hotspot/jtreg/serviceability/jdwp, didn’t find a lot more in that area. I’m hoping/waiting for Chris’ tests then.

 

I agree, I will then remove inStream_endOfInput. If something like that is needed for future developments, it can easily be added again.

 

 

Best regards

Christoph

 

From: [hidden email] [[hidden email]]
Sent: Freitag, 8. Dezember 2017 21:12
To: Langer, Christoph [hidden email]; [hidden email]; Chris Plummer [hidden email]
Subject: Re: RFR (S): 8193258: Better usage of JDWP HEADER SIZE

 

Hi Christoph,

The fix looks good to me.
What tests did you run?


On 12/8/17 07:07, Langer, Christoph wrote:

Hi,

 

Here’s a proposal to clean up the usage of the JDWP header size within the source code of libjdwp.

 

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

WebRev: http://cr.openjdk.java.net/~clanger/webrevs/8193258.0/

 

As for inStream.c, I’m wondering wether inStream_endOfInput shall be removed? It seems to be unused…


I'm inclined to remove it.
Otherwise, it must be:
  417 return (stream->left <= 0);

Thanks,
Serguei
 

Best regards

Christoph