Hi Roman,
The fix looks pretty clean now.
I also like new name of the lock.:)
Just one comment below.
http://cr.openjdk.java.net/~rkennke/JDK-8227269/webrev.06/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c.frames.html
110 if (tag != 0l) {
Hi Sergei,
> The fix looks pretty clean now.
> I also like new name of the lock.:)
Thank you!
> Just one comment below.
>
> http://cr.openjdk.java.net/~rkennke/JDK-8227269/webrev.06/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c.frames.html
>
> 110 if (tag != 0l) {
> 111 return; // Alrea
Hi
Could you please review following fix which change LingeredApp to
prepend vm options to java/vm.test.opts when startApp is used and
provide startAppVmOpts to override options completely.
The intention is to avoid issue like in this bug where test/jtreg
options were ignored by tests. Also
Hi Leonid,
I have briefly looked at the patch, a few comments so far:
test/hotspot/jtreg/serviceability/sa/ClhsdbFlags.java:
- at L#114, could you please call static method using class name (as the
opposite of using instance)? or was it meant to be theApp.runAppVmOpts(vmArgs) ?
test/lib/jdk/te
Hi Lenoid,
Thanks for fixing this.
If you just look at a test case, it's not very obvious what the
difference is between
LingerApp.startApp(myApp, "-XX:Xyz=123");
LingerApp.startAppVmOpts(myApp, "-XX:Xyz=123");
How about renaming startAppVmOpts/runAppVmOpts ->
startAppExactVmOpts/ru
On 2020-03-25 17:40, Igor Ignatyev wrote:
Hi Leonid,
I have briefly looked at the patch, a few comments so far:
test/hotspot/jtreg/serviceability/sa/ClhsdbFlags.java:
- at L#114, could you please call static method using class name (as the
opposite of using instance)? or was it meant to be t
Igor, Stefan
Thank you for feedback, see my comments inline.
On 3/25/20 10:14 AM, Stefan Karlsson wrote:
On 2020-03-25 17:40, Igor Ignatyev wrote:
Hi Leonid,
I have briefly looked at the patch, a few comments so far:
test/hotspot/jtreg/serviceability/sa/ClhsdbFlags.java:
- at L#114, could
Hi Roman,
Regarding the new assert:
105 if (gdata && gdata->assertOn) {
106 // Check this is not already tagged.
107 jlong tag;
108 error = JVMTI_FUNC_PTR(trackingEnv, GetTag)(env, klass, &tag);
109 if (error != JVMTI_ERROR_NONE) {
110 EXIT_E
when you get to this code. If it is ever NULL then there's a bug, and
> the check will hide the bug.
Ok, will remove this.
> Regarding testing, after you do the submit repo testing let me know the
> jobID and I'll do additional testing on it.
I did the submit repo earlier toda
Added Ioi, who also proposed new version of startAppVmOpts.
Please find new webrev:
http://cr.openjdk.java.net/~lmesnik/8240698/webrev.01/
Renamed startAppVmOpts/runAppVmOpts to
"startAppExactJvmOpts/runAppExactJvmOpts" is used. It should make very
clear that this method doesn't use any of t
Thanks for changing the name. Sounds good to me. I leave the full review
to others.
StefanK
On 2020-03-25 20:01, Leonid Mesnik wrote:
Added Ioi, who also proposed new version of startAppVmOpts.
Please find new webrev:
http://cr.openjdk.java.net/~lmesnik/8240698/webrev.01/
Renamed startApp
With the recent fixes in JDK-8241310, JDK-8237746 and JDK-8241073, and
the upcoming fixes to remove the deprecated nashorn and jdk.rmi, the JDK
build is very close to producing no warnings when compiling the Java
classes.
The one remaining sinner is jdk.hotspot.agent. Most of the warnings here
bID and I'll do additional testing on it.
I did the submit repo earlier today, and it came back green:
mach5-one-rkennke-JDK-8227269-2-20200325-1421-9706762
Thanks,
Roman
thanks,
Chris
On 3/25/20 6:00 AM, Roman Kennke wrote:
Hi Sergei,
Hi Leonid,
not related related to your patch (but yet somewhat made more obvious by it),
it seems all (or at least almost all) the tests which use LingeredApp should be
run in "driver" mode as they just orchestrate execution of other JVMs, so
running them w/ main (let alone main/othervm) just w
Hi Magus,
I haven't looked at the changes yet, other to see that there are many
files touched, but after reading below (and only partly understanding
since I don't know this area well), I was wondering if this issue
wouldn't be better served with multiple passes made to fix the warnings.
Star
uld never be NULL
>>> when you get to this code. If it is ever NULL then there's a bug, and
>>> the check will hide the bug.
>> Ok, will remove this.
>>
>>> Regarding testing, after you do the submit repo testing let me know the
>>> jobID and I&#x
Hi Daniil,
On 3/24/20 10:00, Daniil Titov wrote:
Hi Serguei,
It looks like you removed the last call site of DebugServer.main.
Yes. It is correct.
Do we need to remove the DebugServer.java as well?
I was considering this but since it is a public class I think it needs to be
deprec
This new versions looks good to me.
Thanks
- Ioi
On 3/25/20 12:01 PM, Leonid Mesnik wrote:
Added Ioi, who also proposed new version of startAppVmOpts.
Please find new webrev:
http://cr.openjdk.java.net/~lmesnik/8240698/webrev.01/
Renamed startAppVmOpts/runAppVmOpts to
"startAppExactJvmOpt
D and I'll do additional testing on it.
I did the submit repo earlier today, and it came back green:
mach5-one-rkennke-JDK-8227269-2-20200325-1421-9706762
Thanks,
Roman
thanks,
Chris
On 3/25/20 6:00 AM, Roman Kennke wrote:
Hi Sergei,
The fix looks pretty clean now.
I also like new n
On 2020-03-25 20:52, Chris Plummer wrote:
Hi Magus,
I haven't looked at the changes yet, other to see that there are many
files touched, but after reading below (and only partly understanding
since I don't know this area well), I was wondering if this issue
wouldn't be better served with mult
Igor, Stefan, Ioi
Thank you for your feedback.
Filed https://bugs.openjdk.java.net/browse/JDK-8241624 To change @run
main... to @run driver.
Test ClhsdbJstack.java is updated.
Still waiting for review from SVC team.
webrev: http://cr.openjdk.java.net/~lmesnik/8240698/webrev.02/
Leonid
On
> Test ClhsdbJstack.java is updated.
>
now you reduced coverage provided by this test, I actually meant to create a
separate jtreg test description in this test and pass "Xcomp" or "true" (or
anything) as an argument to ClhsdbJstack, and use the value of this argument to
decide if -Xcomp should
The new job finished, its ID is:
mach5-one-rkennke-JDK-8227269-2-20200325-2027-9716289
Thank you,
Roman
> Yes, please submit a new job. I'll start my testing once I see that the
> builds are done.
>
> Chris
>
> On 3/25/20 12:59 PM, Roman Kennke wrote:
>> Hi C
Hi everyone,
As a follow-up to the ongoing review for JDK-8241618, I have also looked
at fixing the deprecation warnings in jdk.hotspot.agent. These fall in
three broad categories:
* Deprecation of the boxing type constructors (e.g. "new Integer(42)").
* Deprecation of java.util.Observer and
On 3/25/20 2:58 PM, Igor Ignatyev wrote:
Test ClhsdbJstack.java is updated.
now you reduced coverage provided by this test, I actually meant to
create a separate jtreg test description in this test and pass "Xcomp"
or "true" (or anything) as an argument to ClhsdbJstack, and use the
value of
> On Mar 25, 2020, at 3:36 PM, Leonid Mesnik wrote:
>
>
>
> On 3/25/20 2:58 PM, Igor Ignatyev wrote:
>>> Test ClhsdbJstack.java is updated.
>>>
>> now you reduced coverage provided by this test, I actually meant to create a
>> separate jtreg test description in this test and pass "Xcomp" or
w job finished, its ID is:
mach5-one-rkennke-JDK-8227269-2-20200325-2027-9716289
Thank you,
Roman
Yes, please submit a new job. I'll start my testing once I see that the
builds are done.
Chris
On 3/25/20 12:59 PM, Roman Kennke wrote:
Hi Chris,
Apparently we can get into classT
27 matches
Mail list logo
