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 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
||
src
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
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
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
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
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 wrote
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
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) {
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
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 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,
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:
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
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
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
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
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
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
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
is 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 <matthias.baes...@sap.com>; Alan Bateman
<alan.bate...@oracle.com>; core-libs-dev@open
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
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
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
to 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 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
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:
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
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/hotspot/src
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
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/src/share/vm
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
://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 JDI
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
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 folders
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
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.
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
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
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
the const 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
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 staffan.lar...@oracle.com
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 staffan.lar...@oracle.com wrote:
Please review the following test fix.
webrev:
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 wrote:
Nice catch
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 one more problem
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
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
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 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.
50 matches
Mail list logo