On 4/17/20 16:52, Mandy Chung wrote:
On 4/17/20 3:51 PM, Chris Plummer wrote:
Hi Mandy,
Thanks for updating the svc specs. Some comments below:
In the JDWP spec update, you changed "JNI signature" to "type
signature" in one place, but left it as "JNI signature" everywhere
else. Should the
LGTM++
Thanks,
Serguei
On 4/28/20 23:28, David Holmes wrote:
Looks good!
Thanks,
David
On 29/04/2020 1:20 pm, Igor Ignatyev wrote:
http://cr.openjdk.java.net/~iignatyev//8244052/webrev.00/
34 lines changed: 0 ins; 11 del; 23 mod;
Hi all,
could you please review this trivial patch?
from
Hi Harold,
The Serviceability part including the tests looks good to me.
I can file a JVMTI enhancement on the jvmtiRedefineClasses.cpp
refactoring if you are okay with it.
Thanks,
Serguei
On 5/18/20 22:26, David Holmes wrote:
Hi Harold,
Adding serviceability-dev for the serviceability rel
On 5/19/20 09:46, Harold Seigel wrote:
That sounds good!
Thanks, Harold
On 5/19/2020 11:53 AM, serguei.spit...@oracle.com wrote:
Hi Harold,
The Serviceability part including the tests looks good to me.
I can file a JVMTI enhancement on the jvmtiRedefineClasses.cpp
refactoring if you are
Hi David,
On 5/19/20 19:31, David Holmes wrote:
Hi Serguei,
On 20/05/2020 1:49 am, serguei.spit...@oracle.com wrote:
Hi Harold and David,
Just one comment.
We could save on the CSR's for:
make/data/jdwp/jdwp.spec
|| src/jdk.jdi/share/classes/com/sun/jdi/VirtualMachine.java
||
Hi Mandy,
This looks good from the Serviceability point of view.
> No change is made in JNI. JNI could be considered to disallow
modification of
> final fields in records and hidden classes (static and instance fields).
> But JNI has superpower and the current spec already allows to modify
>
Hi Jon,
It looks good to me.
Thanks,
Serguei
On 6/21/19 11:58, Jonathan Gibbons wrote:
Please review a fix for an accessibility issue reported by Axe in
src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html
The issue is a conflict between an explicit `role="main">...`` specified in
Hi Brent,
The updated webrev looks good to me.
At least, I do not see any issues.
Thanks,
Serguei
On 9/5/19 17:09, Brent Christian wrote:
Hi, David
On 9/5/19 12:52 AM, David Holmes wrote:
Good to see this all come together :)
:)
So to clarify for others any current caller to
find_class
Hi David,
The fix looks good to me.
I did not pay much attention to the Graal related changes though.
The test coverage for Serviceability is complete.
Running java/lang/instrument tests is not necessary.
Thanks,
Serguei
On 10/29/19 00:42, David Holmes wrote:
Bug: https://bugs.openjdk.java.ne
Hi Christoph,
The fix looks good to me.
I'd recommend to run the jdk_instrument and vmTestbase_nsk_jvmti tests.
Also, it can be safe to run:
open/test/hotspot/jtreg/serviceability/jvmti
open/test/hotspot/jtreg/runtime/cds/appcds
open/test/hotspot/jtreg/runtime/BootClassAppendProp
Thanks,
S
these tests on several platforms.
Best regards
Christoph
-Original Message-
From: serguei.spit...@oracle.com
Sent: Mittwoch, 20. November 2019 03:21
To: Langer, Christoph ; core-libs-
d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net; gerard
ziemski
Subject: Re: RFR: 8234185
Hi Mandy and Chris,
On 3/29/20 19:17, Mandy Chung wrote:
On 3/27/20 8:51 PM, Chris Plummer wrote:
Hi Mandy,
A couple of very minor nits in the jvmtiRedefineClasses.cpp comments:
153 // classes for primitives, arrays, hidden and vm unsafe
anonymous classes
154 // cannot be redefi
Hi Mandy,
I have just one comment so far.
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03/src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp.frames.html
356 void add_classes(LoadedClassInfo* first_class, int num_classes,
bool has_class_mirror_holder) {
3
On 3/30/20 02:30, serguei.spit...@oracle.com wrote:
Hi Mandy,
I have just one comment so far.
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03/src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp.frames.html
356 void add_classes(LoadedClassInfo
On 4/6/20 11:54, Mandy Chung wrote:
On 4/6/20 9:56 AM, serguei.spit...@oracle.com wrote:
The suggested fix is:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/valhalla-jdi-regression-8242166.1/
This patch looks okay. I'll include in my local patch.
On 4/6/20 11:00 AM, Chris Plummer
Hi Frederik,
Your fix is already in a good shape!
src/share/vm/services/management.cpp:
It is better to have different diagnostic messages at lines 2181 and 2186
so that it is easy to find out what of the two lines caused an exception.
The messages would be better to be more specific.
T
Mandy,
It looks good.
Just a question below.
|| *src/share/classes/java/lang/management/ManagementFactory.java*
596 String intfName = mxbeanInterface.getName();
599 " is not an instance of " + mxbeanInterface);
Did you want this?:
596 String intfName
Looks good.
Thanks,
Serguei
On 8/23/12 12:33 PM, Mandy Chung wrote:
On 8/23/2012 11:58 AM, Alan Bateman wrote:
On 23/08/2012 18:43, Mandy Chung wrote:
This change is to bring the jdk modularization changes from jigsaw
repo [1]
to jdk8. This allows the jdk modularization changes to be expos
I was thinking about the same.
But a CCC request is needed for such a change in public API.
It can be done separately if any other API changes are necessary.
Thanks,
Serguei
On 8/23/12 6:27 PM, David Holmes wrote:
Hi Mandy,
While I understand what you are doing and that it "works" it is far
Hi Mandy,
It looks pretty good to me.
At least, I do not see any obvious issues.
Just some minor comments below.
webrev.03/hotspot/src/share/vm/classfile/javaClasses.cpp
2128 Symbol* java_lang_StackFrameInfo::get_file_name(Handle stackFrame,
InstanceKlass* holder) {
2129 if (MemberNameInS
Somehow some of the formatting in my reply is gone.
I'm trying to fix it below...
Thanks,
Serguei
On 11/20/15 01:59, serguei.spit...@oracle.com wrote:
Hi Mandy,
It looks pretty good to me.
At least, I do not see any obvious issues.
Just some minor comments below.
webrev.03/hotspo
On 11/20/15 08:39, Mandy Chung wrote:
On Nov 20, 2015, at 1:59 AM, serguei.spit...@oracle.com wrote:
The 'int bci' is not used above.
This is weird. Daniel caught that and I took that line out already.
http://cr.openjdk.java.net/~mchung/jdk9/jep259/webrev.03-delta/hotspot/sr
Looks good.
Thanks,
Serguei
On 11/24/15 14:37, Mandy Chung wrote:
On Nov 24, 2015, at 2:20 PM, Daniel D. Daugherty
wrote:
You use both 'this.anchor' and 'anchor'. Seems inconsistent.
Oh yeah. I took out “this.” from it.
diff --git a/src/java.base/share/classes/java/lang/StackStreamFacto
Reviewed
Thanks,
Serguei
On 11/19/14 12:49 AM, Chris Plummer wrote:
I've update the webrev to add STACK_SIZE_MINIMUM in place of the 32k
references, and also moved the test from hotspot/test/runtime to
jdk/test/tools/launcher as David requested. That required some
adjustments to the test scri
The fix still looks good to me.
Thanks,
Serguei
On 12/1/14 6:39 PM, Chris Plummer wrote:
Sorry about the long delay in getting back to this. I ran into two
separate JPRT issues that were preventing me from testing these
changes, plus I was on vacation last week. Here's an updated webrev.
I'm
It still looks good to me too. :)
Thanks,
Serguei
On 12/4/14 3:46 PM, David Holmes wrote:
Looks good to me too Chris - sorry for the delay getting back to you.
But at least Kumar spotted all the typos :)
David
On 4/12/2014 6:12 PM, Chris Plummer wrote:
On 12/3/14 4:56 AM, Alan Bateman wrote
Chris,
It looks good in general.
I'd suggest to rename the folder:
|| test/com/sun/jdi/CDSJDITests
to:
test/com/sun/jdi/cds
There is no need to spell "JDI" as it is already a sub-folder of the
com/sun/jdi
and there is no need to spell "Tests" too as it is in the test repo.
Also, all the fold
It looks good to me.
Reviewed all together.
Thanks,
Serguei
Thanks,
Serguei
On 6/2/15 8:20 PM, Chris Plummer wrote:
Please review the following:
Webrev: http://cr.openjdk.java.net/~cjplummer/8054386/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8081771
This review only concerns th
ad:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-June/033892.html
thanks,
Chris
On 6/3/15 1:40 AM, serguei.spit...@oracle.com wrote:
Chris,
It looks good in general.
I'd suggest to rename the folder:
|| test/com/sun/jdi/CDSJDITests
to:
test/com/sun/jdi/cds
There is no need to spell
I guess, there is no need to re-review.
It looks good anyway.
Thanks,
Serguei
On 6/4/15 4:32 PM, Chris Plummer wrote:
Hi David,
Here's an updated webrev:
http://cr.openjdk.java.net/~cjplummer/8054386/webrev.04/
thanks,
Chris
On 6/3/15 11:29 PM, David Holmes wrote:
Hi Chris,
Hotspot chan
Hi Bob,
It looks good to me.
Thanks,
Serguei
On 3/21/19 10:12, Bob Vandette wrote:
Please review this fix for a container test that fails on some Linux
distributions.
The test fails to see the Memory Fail count metric value increase. This is
caused by
the fact that we are allowing ergonom
David,
The change looks good but you already have enough reviewers. :)
Just wanted to thank you for taking steps on this issue.
Thanks,
Serguei
On 5/18/16 21:52, David Holmes wrote:
Not sure who really owns this file so cc'ing core-libs and
serviceability.
bug: https://bugs.openjdk.java.net
Hi Dan,
The fix looks good to me.
Nice, you have caught it.
Do you want this fixed in 10 or 11?
I thought that the jdk/hs is for 11 now.
Is it correct?
Thanks,
Serguei
On 12/7/17 09:09, Daniel D. Daugherty wrote:
Roger,
Thanks for the review!
Dan
P.S.
I'm planning to push this fix to jdk/
On 12/7/17 10:08, serguei.spit...@oracle.com wrote:
Hi Dan,
The fix looks good to me.
Nice, you have caught it.
Do you want this fixed in 10 or 11?
I thought that the jdk/hs is for 11 now.
Is it correct?
Never mind.
I've just found a message from Jesper the jdk/hs is used for 10 pushes
David,
It looks good.
Thanks,
Serguei
On 12/20/17 02:22, David Holmes wrote:
Before we can update to JDK version 11, jtreg needs to be updated to
recognize that release value - which is happening in jtreg 4.2 b11. So
we then need to ensure that jtreg 4.2 b11 is used by updating the
required
Hi Igor,
I do not see any issues with this refactoring.
Thanks,
Serguei
On 9/6/18 09:19, Igor Ignatyev wrote:
JC,
thanks for your review!
Core-libs team,
as the majority of changed tests are core-libs tests, I'd really appreciate if
someone from core-libs (preferably a Reviewer) could revie
Hi Igor,
This looks good to me.
A couple of questions below.
http://cr.openjdk.java.net/~iignatyev//8176176/webrev.01/test/com/sun/jdi/InvokeHangTest.java.udiff.html
- * @modules jdk.jdi
* @library /test/lib
+ * @modules java.management
+ * jdk.jdiShould the jdk.jdi be removed from this
specify @modules tag in them, and
because @modules in a test overrides modules from TEST.properties, jdk.jdi
module should be mentioned in the tests.
— Igor
On Mar 15, 2017, at 9:53 PM, serguei.spit...@oracle.com wrote:
Hi Igor,
This looks good to me.
A couple of questions below.
http
Hi Amy,
It looks good to me.
Thanks,
Serguei
On 3/30/17 22:42, Amy Lu wrote:
I'm still waiting for reviewer's feedback for the TEST.groups update:
jdk/internal/loader (add to jdk_lang)
jdk/internal/util (add to jdk_util_other)
jdk/internal/agent (add to jdk_management)
(Security part has
Hi Kumar,
It looks fine to me too.
Thanks,
Serguei
On 5/2/17 16:23, Mandy Chung wrote:
Looks fine to me.
Mandy
On May 2, 2017, at 12:40 PM, Kumar Srinivasan
wrote:
Hello,
Please review changes to make jdk.jdi HTML5 friendly,
table cell padding has not been addressed and will be fixed
s
Hi Matthias,
The fix looks good to me.
It must be tested at least with the JTreg com/sun/jdi tests.
Do you need a sponsor?
Thanks,
Serguei
On 5/16/17 03:21, Baesken, Matthias wrote:
Sure, I forward it to serviceability-dev .
-Original Message-
From: Alan Bateman [mailto:alan.bate
in JDK not HS .
Best regards, Matthias
-Original Message-
From: serguei.spit...@oracle.com [mailto:serguei.spit...@oracle.com]
Sent: Freitag, 19. Mai 2017 02:18
To: Baesken, Matthias ; Alan Bateman
; core-libs-dev@openjdk.java.net;
serviceability-...@openjdk.java.net
Cc: Simonis
Hi Staffan,
Thank you for this discovery!
It looks good, but I have a couple of comments.
It seems, there is one more problem in this code:
68 /* check for NULL path */
69 if (p == pathname) {
70 continue;<== Endless loop if we hit this line
71 }
Do
loop" to track this new problem and I am working on a fix.
Thanks,
/Staffan
On 5 mar 2013, at 20:26, serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com> wrote:
Hi Staffan,
Thank you for this discovery!
It looks good, but I have a couple of comments.
It seems, there is
Looks good.
Thanks,
Serguei
On 3/7/13 12:10 AM, Staffan Larsen wrote:
Adding core-libs-dev and re-asking for a review.
Thanks,
/Staffan
On 27 feb 2013, at 15:59, Staffan Larsen wrote:
Please review the following test fix.
webrev: http://cr.openjdk.java.net/~sla/8006637/webrev.00/
bug: htt
i and hprof tests passes with this change.
Thanks,
/Staffan
On 6 mar 2013, at 20:48, serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com> wrote:
Staffan,
Thank you for the confirmation and taking care about this issue!
Thanks,
Serguei
On 3/6/13 11:36 AM, Staffan Larsen w
t qualifier to some of the parameters.
http://cr.openjdk.java.net/~sla/8009558/webrev.02/
<http://cr.openjdk.java.net/%7Esla/8009558/webrev.02/>
All of the jdi and hprof tests passes with this change.
Thanks,
/Staffan
On 6 mar 2013, at 20:48, serguei.spit...@oracle.com
<mailto:ser
It is just a remainder that I reviewed this as well (the fix is good too).
My email is attached.
Thanks,
Serguei
On 3/18/13 7:14 AM, Staffan Larsen wrote:
I still need an official Review for this change.
Thanks,
Staffan
On 7 mar 2013, at 09:10, Staffan Larsen wrote:
Adding core-libs-dev a
Hi Mikael,
It looks good.
Thank you for figuring out how to make it more stable!
BTW, the webrev frames mode does not work.
Thanks,
Serguei
On 4/17/13 6:03 AM, Mikael Auno wrote:
Hi, I'd like some reviews on
http://cr.openjdk.java.net/~nloodin/8009681/webrev.01/ for JDK-8009681
(http://bugs.
Hi Bill,
I've already had a chance to read your webrev several weeks ago, but
need more time.
Thanks,
Serguei
On 7/3/13 2:17 PM, BILL PITTORE wrote:
These changes address bug 8014135 which adds support for statically
linked agents in the VM. This is a followup to the recent JNI spec
changes
Coleen,
The fix looks good.
It is nice you've come up with this.
Thanks,
Serguei
On 10/3/13 11:02 AM, Coleen Phillimore wrote:
Summary: Redefined class in stack trace may not be found by
method_idnum so handle null.
This is a simple change. I had another change to save the method name
(as
51 matches
Mail list logo
