Re: RFR: 8206438: com/sun/jdi/FieldWatchpoints.java timeout intermittently
On Tue, 21 Sep 2021 07:40:01 GMT, Fairoz Matte wrote: > 8206438: com/sun/jdi/FieldWatchpoints.java timeout intermittently Thanks Leonid and Chris for looking into this. I couldn't reproduce the issue locally and none of the issues observed on mach5 have the active links for logs https://mach5.us.oracle.com/mdash/testHistory?search=com%2Fsun%2Fjdi%2FFieldWatchpoints.java%20AND%20products.JDK.profile%3A*windows*%20AND%20status%3Afailed%20AND%20reasons.details%3A*time* During #jdk-ur-gatekeeping We observed for the below failure, test timedout for 160-170seconds. https://mach5.us.oracle.com/mdash/bugHistory?search=testName%3A*com%2Fsun%2Fjdi%2FFieldWatchpoints.java So there is no evidence now to prove that the problem will be fixed by increasing the timeout. If this is not a right way, I will close the bug and it can be reopened once we observe the timeout issue again. Thanks, - PR: https://git.openjdk.java.net/jdk/pull/5598
RFR: 8206438: com/sun/jdi/FieldWatchpoints.java timeout intermittently
8206438: com/sun/jdi/FieldWatchpoints.java timeout intermittently - Commit messages: - 8206438: com/sun/jdi/FieldWatchpoints.java timeout intermittently Changes: https://git.openjdk.java.net/jdk/pull/5598/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5598=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8206438 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5598.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5598/head:pull/5598 PR: https://git.openjdk.java.net/jdk/pull/5598
Re: RFR: 8273575: memory leak in appendBootClassPath(), paths must be deallocated
On Wed, 15 Sep 2021 01:05:10 GMT, Serguei Spitsyn wrote: > The memory allocated and hold in the paths local variable of function > appendBootClassPath() is never deallocated: > splitPathList(pathList, , ); > So, it is a memory leak which needs to be fixed. > The fix is to add one line at the end of function: > free(paths); Changes looks good to me. - PR: https://git.openjdk.java.net/jdk/pull/5516
Integrated: 8221503: vmTestbase/nsk/jdb/eval/eval001/eval001.java fails with: com.sun.jdi.InvalidTypeException: Can't assign double[][][] to double[][][]
On Fri, 23 Apr 2021 15:03:42 GMT, Fairoz Matte wrote:
> findComponentType() logic is wrong. In findComponentType() method, We always
> get vm.classesByName() retruns empty list
> list = vm.classesByName(parser.typeName());
> We have "parser.typeName()" retruns " double[][]"
> vm.classesByName("") is expecting the fully qualified name example
> "java.lang.Double"
> This always returns empty list, resulting into ClassNotLoadedException as it
> assumes the Component class has not yet been loaded, hence the test case
> fails.
>
> There was a suggested fix from Egor Ushakov from JetBrains, I am proposing
> the same to get this fix. I have verified the patch with required testing it
> works fine.
This pull request has now been integrated.
Changeset: 82768d9a
Author:Fairoz Matte
Committer: Serguei Spitsyn
URL:
https://git.openjdk.java.net/jdk/commit/82768d9a31edcfe5b27e75d681d3592c8f4a2ece
Stats: 41 lines in 3 files changed: 0 ins; 36 del; 5 mod
8221503: vmTestbase/nsk/jdb/eval/eval001/eval001.java fails with:
com.sun.jdi.InvalidTypeException: Can't assign double[][][] to double[][][]
Reviewed-by: cjplummer, sspitsyn
-
PR: https://git.openjdk.java.net/jdk/pull/3658
Re: RFR: 8221503: vmTestbase/nsk/jdb/eval/eval001/eval001.java fails with: com.sun.jdi.InvalidTypeException: Can't assign double[][][] to double[][][] [v2]
On Thu, 29 Apr 2021 07:58:26 GMT, Fairoz Matte wrote:
>> findComponentType() logic is wrong. In findComponentType() method, We always
>> get vm.classesByName() retruns empty list
>> list = vm.classesByName(parser.typeName());
>> We have "parser.typeName()" retruns " double[][]"
>> vm.classesByName("") is expecting the fully qualified name example
>> "java.lang.Double"
>> This always returns empty list, resulting into ClassNotLoadedException as it
>> assumes the Component class has not yet been loaded, hence the test case
>> fails.
>>
>> There was a suggested fix from Egor Ushakov from JetBrains, I am proposing
>> the same to get this fix. I have verified the patch with required testing it
>> works fine.
>
> Fairoz Matte has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Update ArrayReferenceImpl.java
> - Update ArrayTypeImpl.java
Hi Serguei, thanks for the review.
-
PR: https://git.openjdk.java.net/jdk/pull/3658
Re: RFR: 8221503: vmTestbase/nsk/jdb/eval/eval001/eval001.java fails with: com.sun.jdi.InvalidTypeException: Can't assign double[][][] to double[][][] [v2]
On Thu, 29 Apr 2021 07:58:26 GMT, Fairoz Matte wrote:
>> findComponentType() logic is wrong. In findComponentType() method, We always
>> get vm.classesByName() retruns empty list
>> list = vm.classesByName(parser.typeName());
>> We have "parser.typeName()" retruns " double[][]"
>> vm.classesByName("") is expecting the fully qualified name example
>> "java.lang.Double"
>> This always returns empty list, resulting into ClassNotLoadedException as it
>> assumes the Component class has not yet been loaded, hence the test case
>> fails.
>>
>> There was a suggested fix from Egor Ushakov from JetBrains, I am proposing
>> the same to get this fix. I have verified the patch with required testing it
>> works fine.
>
> Fairoz Matte has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Update ArrayReferenceImpl.java
> - Update ArrayTypeImpl.java
Hi Chris, thanks for the rreview.
> test/hotspot/jtreg/vmTestbase/nsk/jdb
> test/hotspot/jtreg/serviceability/jdwp
> test/hotspot/jtreg/vmTestbase/nsk/jdwp
> test/hotspot/jtreg/vmTestbase/nsk/jdi
> test/jdk/com/sun/jdi
I have verified tier1 to tier8 and didn't find any issues.
Could you please sponsor this push for me.
Thanks,
-
PR: https://git.openjdk.java.net/jdk/pull/3658
Re: RFR: 8221503: vmTestbase/nsk/jdb/eval/eval001/eval001.java fails with: com.sun.jdi.InvalidTypeException: Can't assign double[][][] to double[][][] [v2]
On Thu, 29 Apr 2021 04:31:35 GMT, Chris Plummer wrote:
>> Hi Chris,
>>
>> Thanks for looking into this,
>>
>>>Do we even need findComponentType() any more? Isn't
>>>ReferenceTypeImpl.findType() sufficient.
>>
>> We still need findComponentType(),
>> Difference between findType() and findComponentType() is that,
>> findComponentType() tries to get the list of ReferenceType from the
>> "vm.classesByName". In case list is empty, it explicitly throws
>> ClassNotLoadedException.
>> This exception check is required in validateAssignment(ValueContainer
>> destination) call from ObjectReferenceImpl.java.
>>
>> https://github.com/openjdk/jdk/blob/master/src/jdk.jdi/share/classes/com/sun/tools/jdi/ObjectReferenceImpl.java#L579
>>
>>
>>
>> I have modified the method a bit to handle that scenario
>>
>>
>> diff --git a/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java
>> b/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java
>> index e544c81ae3e..54deba43894 100644
>> --- a/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java
>> +++ b/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java
>> @@ -95,20 +95,13 @@ public class ArrayTypeImpl extends ReferenceTypeImpl
>> JNITypeParser sig = new JNITypeParser(signature);
>> if (sig.isReference()) {
>> // It's a reference type
>> -JNITypeParser parser = new JNITypeParser(componentSignature());
>> -List list = vm.classesByName(parser.typeName());
>> -Iterator iter = list.iterator();
>> -while (iter.hasNext()) {
>> -ReferenceType type = iter.next();
>> -ClassLoaderReference cl = type.classLoader();
>> -if ((cl == null)?
>> - (classLoader() == null) :
>> - (cl.equals(classLoader( {
>> -return type;
>> -}
>> + try {
>> + Type componentType = findType(componentSignature());
>> + return componentType;
>> + // Component class has not yet been loaded
>> + } catch (ClassNotLoadedException ex) {
>> + throw new ClassNotLoadedException(componentTypeName());
>> }
>> -// Component class has not yet been loaded
>> -throw new ClassNotLoadedException(componentTypeName());
>> } else {
>> // It's a primitive type
>> return vm.primitiveTypeMirror(sig.jdwpTag());```
>>
>> Thanks,
>
>> > Do we even need findComponentType() any more? Isn't
>> > ReferenceTypeImpl.findType() sufficient.
>>
>> We still need findComponentType(),
>> Difference between findType() and findComponentType() is that,
>> findComponentType() tries to get the list of ReferenceType from the
>> "vm.classesByName". In case list is empty, it explicitly throws
>> ClassNotLoadedException.
>> This exception check is required in validateAssignment(ValueContainer
>> destination) call from ObjectReferenceImpl.java.
>
> I'm not sure what you mean by this. After your changes, this is all
> `findComponentType()` does:
>
>
> Type findComponentType(String signature) throws ClassNotLoadedException {
> return findType(signature);
> }
>
>
> And `findType()` has the exact same signature, including the `throws`:
>
> `Type findType(String signature) throws ClassNotLoadedException {`
>
>
> So my suggestion is to get rid of `findComponentType()` and just have current
> users call `findType()` instead.
Thanks Chris, for the review comments.
I have modified the suggested changes, kindly review.
Thanks,
-
PR: https://git.openjdk.java.net/jdk/pull/3658
Re: RFR: 8221503: vmTestbase/nsk/jdb/eval/eval001/eval001.java fails with: com.sun.jdi.InvalidTypeException: Can't assign double[][][] to double[][][] [v2]
> findComponentType() logic is wrong. In findComponentType() method, We always
> get vm.classesByName() retruns empty list
> list = vm.classesByName(parser.typeName());
> We have "parser.typeName()" retruns " double[][]"
> vm.classesByName("") is expecting the fully qualified name example
> "java.lang.Double"
> This always returns empty list, resulting into ClassNotLoadedException as it
> assumes the Component class has not yet been loaded, hence the test case
> fails.
>
> There was a suggested fix from Egor Ushakov from JetBrains, I am proposing
> the same to get this fix. I have verified the patch with required testing it
> works fine.
Fairoz Matte has updated the pull request incrementally with two additional
commits since the last revision:
- Update ArrayReferenceImpl.java
- Update ArrayTypeImpl.java
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/3658/files
- new: https://git.openjdk.java.net/jdk/pull/3658/files/17fe300c..37170d21
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=3658=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=3658=00-01
Stats: 17 lines in 2 files changed: 0 ins; 13 del; 4 mod
Patch: https://git.openjdk.java.net/jdk/pull/3658.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/3658/head:pull/3658
PR: https://git.openjdk.java.net/jdk/pull/3658
Re: RFR: 8221503: vmTestbase/nsk/jdb/eval/eval001/eval001.java fails with: com.sun.jdi.InvalidTypeException: Can't assign double[][][] to double[][][]
On Tue, 27 Apr 2021 23:47:26 GMT, Chris Plummer wrote:
>> findComponentType() logic is wrong. In findComponentType() method, We always
>> get vm.classesByName() retruns empty list
>> list = vm.classesByName(parser.typeName());
>> We have "parser.typeName()" retruns " double[][]"
>> vm.classesByName("") is expecting the fully qualified name example
>> "java.lang.Double"
>> This always returns empty list, resulting into ClassNotLoadedException as it
>> assumes the Component class has not yet been loaded, hence the test case
>> fails.
>>
>> There was a suggested fix from Egor Ushakov from JetBrains, I am proposing
>> the same to get this fix. I have verified the patch with required testing it
>> works fine.
>
> src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java line 94:
>
>> 92: */
>> 93: Type findComponentType(String signature) throws
>> ClassNotLoadedException {
>> 94: return findType(signature);
>
> Do we even need `findComponentType()` any more? Isn't
> `ReferenceTypeImpl.findType()` sufficient.
>
> The comment above `findComponentType()` is kind of explicit as to why it was
> needed. Are you sure none of that still applies, and there isn't some edge
> case that `findType()` is not covering?
Hi Chris,
Thanks for looking into this,
>Do we even need findComponentType() any more? Isn't
>ReferenceTypeImpl.findType() sufficient.
We still need findComponentType(),
Difference between findType() and findComponentType() is that,
findComponentType() tries to get the list of ReferenceType from the
"vm.classesByName". In case list is empty, it explicitly throws
ClassNotLoadedException.
This exception check is required in validateAssignment(ValueContainer
destination) call from ObjectReferenceImpl.java.
https://github.com/openjdk/jdk/blob/master/src/jdk.jdi/share/classes/com/sun/tools/jdi/ObjectReferenceImpl.java#L579
diff --git a/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java
b/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java
index e544c81ae3e..54deba43894 100644
--- a/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java
+++ b/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java
@@ -95,20 +95,13 @@ public class ArrayTypeImpl extends ReferenceTypeImpl
JNITypeParser sig = new JNITypeParser(signature);
if (sig.isReference()) {
// It's a reference type
-JNITypeParser parser = new JNITypeParser(componentSignature());
-List list = vm.classesByName(parser.typeName());
-Iterator iter = list.iterator();
-while (iter.hasNext()) {
-ReferenceType type = iter.next();
-ClassLoaderReference cl = type.classLoader();
-if ((cl == null)?
- (classLoader() == null) :
- (cl.equals(classLoader( {
-return type;
-}
+ try {
+ Type componentType = findType(componentSignature());
+ return componentType;
+ // Component class has not yet been loaded
+ } catch (ClassNotLoadedException ex) {
+ throw new ClassNotLoadedException(componentTypeName());
}
-// Component class has not yet been loaded
-throw new ClassNotLoadedException(componentTypeName());
} else {
// It's a primitive type
return vm.primitiveTypeMirror(sig.jdwpTag());
Thanks,
-
PR: https://git.openjdk.java.net/jdk/pull/3658
RFR: 8221503: vmTestbase/nsk/jdb/eval/eval001/eval001.java fails with: com.sun.jdi.InvalidTypeException: Can't assign double[][][] to double[][][]
findComponentType() logic is wrong. In findComponentType() method, We always
get vm.classesByName() retruns empty list
list = vm.classesByName(parser.typeName());
We have "parser.typeName()" retruns " double[][]"
vm.classesByName("") is expecting the fully qualified name example
"java.lang.Double"
This always returns empty list, resulting into ClassNotLoadedException as it
assumes the Component class has not yet been loaded, hence the test case fails.
There was a suggested fix from Egor Ushakov from JetBrains, I am proposing the
same to get this fix. I have verified the patch with required testing it works
fine.
-
Commit messages:
- 8221503: vmTestbase/nsk/jdb/eval/eval001/eval001.java fails with:
com.sun.jdi.InvalidTypeException: Can't assign double[][][] to double[][][]
Changes: https://git.openjdk.java.net/jdk/pull/3658/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3658=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8221503
Stats: 25 lines in 2 files changed: 0 ins; 23 del; 2 mod
Patch: https://git.openjdk.java.net/jdk/pull/3658.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/3658/head:pull/3658
PR: https://git.openjdk.java.net/jdk/pull/3658
RE: RFR(s): 8248295: serviceability/jvmti/CompiledMethodLoad/Zombie.java failure with Graal
Thanks Vladimir and Serguei for the reviews. Thanks, Fairoz > -Original Message- > From: Serguei Spitsyn > Sent: Thursday, August 20, 2020 1:45 AM > To: Vladimir Kozlov ; Fairoz Matte > ; hotspot-compiler-...@openjdk.java.net; > serviceability-dev@openjdk.java.net > Cc: Coleen Phillimore > Subject: Re: RFR(s): 8248295: > serviceability/jvmti/CompiledMethodLoad/Zombie.java failure with Graal > > Hi Fairoz, > > LGTM++ > > Thanks, > Serguei > > > On 8/19/20 09:38, Vladimir Kozlov wrote: > > Looks good. > > > > Thanks, > > Vladimir K > > > > On 8/19/20 5:30 AM, Fairoz Matte wrote: > >> Hi Vladimir, > >> > >> Thanks for the review. > >> > >>> I would suggest to run test with -XX:+PrintCodeCache flag which > >>> prints CodeCache usage on exit. > >>> > >>> Also add '-ea -esa' flags - some runs failed with them because they > >>> increase Graal's methods size. > >>> > >>> Running test with immediately caused OOM error on my local linux > >>> machine: > >>> > >>> '-server -ea -esa -XX:+TieredCompilation -XX:+PrintCodeCache - > >>> XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI - > >>> XX:+UseJVMCICompiler -Djvmci.Compiler=graal' > >>> > >>> With -XX:ReservedCodeCacheSize=30m I got: > >>> > >>> [11.217s][warning][codecache] CodeCache is full. Compiler has been > >>> disabled. > >>> [11.217s][warning][codecache] Try increasing the code cache size > >>> using - XX:ReservedCodeCacheSize= > >>> > >>> With -XX:ReservedCodeCacheSize=50m I got this output: > >> > >> Further testing with PrintCodeCache, ReservedCodeCacheSize = 50MB is > >> the safe one to use. > >> > >>> > >>> CodeCache: size=51200Kb used=34401Kb max_used=34401Kb > free=16798Kb > >>> > >>> May be you need to set it to 35m or better to 50m to be safe. > >>> > >>> Note, without Graal test uses only 5.5m: > >>> > >>> CodeCache: size=20480Kb used=5677Kb max_used=5688Kb > free=14803Kb > >>> > >>> - > >>> > >>> I also forgot to ask you to update test's Copyright year. > >> > >> I have updated the copyright year. > >> Updated webrev for the reference - > >> http://cr.openjdk.java.net/~fmatte/8248295/webrev.01/ > >> > >> Thanks, > >> Fairoz > >>> > >>> Regards, > >>> Vladimir K > >>> > >>> On 8/18/20 1:10 AM, Fairoz Matte wrote: > >>>> Hi Vladimir, > >>>> > >>>> Thanks for looking into. > >>>> This is intermittent crash, and is reproducible in windows debug > >>>> build > >>> environment. Below is the testing performed. > >>>> > >>>> 1. Issues observed 7/100 runs, ReservedCodeCacheSize=20m with "- > >>> XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI - > >>> XX:+UseJVMCICompiler" > >>>> 2. Issues observed 0/300 runs, ReservedCodeCacheSize=30m with "- > >>> XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI - > >>> XX:+UseJVMCICompiler" > >>>> > >>>> Thanks, > >>>> Fairoz > >>>> > >>>>> -Original Message- > >>>>> From: Vladimir Kozlov > >>>>> Sent: Monday, August 17, 2020 11:22 PM > >>>>> To: Fairoz Matte ; hotspot-compiler- > >>>>> d...@openjdk.java.net; serviceability-dev@openjdk.java.net > >>>>> Cc: Coleen Phillimore ; Dean Long > >>>>> > >>>>> Subject: Re: RFR(s): 8248295: > >>>>> serviceability/jvmti/CompiledMethodLoad/Zombie.java failure with > >>>>> Graal > >>>>> > >>>>> Hi Fairoz, > >>>>> > >>>>> How you determine that +10Mb is enough with Graal? > >>>>> > >>>>> Thanks, > >>>>> Vladimir > >>>>> > >>>>> On 8/17/20 5:46 AM, Fairoz Matte wrote: > >>>>>> Hi, > >>>>>> > >>>>>> > >>>>>> > >>>>>> Please review this small test change to work with Graal. > >>>>>> > >>>>>> > >>>>>> > >>>>>> Background: > >>>>>> > >>>>>> Graal require more code cache compared to c1/c2. but the test > >>>>>> case always > >>>>> set it to 20MB. This may not be sufficient when running graal. > >>>>>> > >>>>>> Default configuration for ReservedCodeCacheSize = 250MB > >>>>>> > >>>>>> With graal enabled, ReservedCodeCacheSize = 350MB > >>>>>> > >>>>>> > >>>>>> > >>>>>> Either we can modify the framework to honor > ReservedCodeCacheSize > >>>>>> for > >>>>> graal or just update the testcase. > >>>>>> > >>>>>> There are not many test cases they rely on ReservedCodeCacheSize > >>>>>> or > >>>>> InitialCodeCacheSize. So the fix prefer the later one. > >>>>>> > >>>>>> > >>>>>> > >>>>>> JBS - https://bugs.openjdk.java.net/browse/JDK-8248295 > >>>>>> > >>>>>> Webrev - http://cr.openjdk.java.net/~fmatte/8248295/webrev.00/ > >>>>>> > >>>>>> > >>>>>> > >>>>>> Thanks, > >>>>>> > >>>>>> Fairoz > >>>>>> > >>>>>> > >>>>>> >
RE: RFR(s): 8248295: serviceability/jvmti/CompiledMethodLoad/Zombie.java failure with Graal
Hi Vladimir, Thanks for the review. > I would suggest to run test with -XX:+PrintCodeCache flag which prints > CodeCache usage on exit. > > Also add '-ea -esa' flags - some runs failed with them because they increase > Graal's methods size. > > Running test with immediately caused OOM error on my local linux machine: > > '-server -ea -esa -XX:+TieredCompilation -XX:+PrintCodeCache - > XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI - > XX:+UseJVMCICompiler -Djvmci.Compiler=graal' > > With -XX:ReservedCodeCacheSize=30m I got: > > [11.217s][warning][codecache] CodeCache is full. Compiler has been > disabled. > [11.217s][warning][codecache] Try increasing the code cache size using - > XX:ReservedCodeCacheSize= > > With -XX:ReservedCodeCacheSize=50m I got this output: Further testing with PrintCodeCache, ReservedCodeCacheSize = 50MB is the safe one to use. > > CodeCache: size=51200Kb used=34401Kb max_used=34401Kb free=16798Kb > > May be you need to set it to 35m or better to 50m to be safe. > > Note, without Graal test uses only 5.5m: > > CodeCache: size=20480Kb used=5677Kb max_used=5688Kb free=14803Kb > > - > > I also forgot to ask you to update test's Copyright year. I have updated the copyright year. Updated webrev for the reference - http://cr.openjdk.java.net/~fmatte/8248295/webrev.01/ Thanks, Fairoz > > Regards, > Vladimir K > > On 8/18/20 1:10 AM, Fairoz Matte wrote: > > Hi Vladimir, > > > > Thanks for looking into. > > This is intermittent crash, and is reproducible in windows debug build > environment. Below is the testing performed. > > > > 1. Issues observed 7/100 runs, ReservedCodeCacheSize=20m with "- > XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI - > XX:+UseJVMCICompiler" > > 2. Issues observed 0/300 runs, ReservedCodeCacheSize=30m with "- > XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI - > XX:+UseJVMCICompiler" > > > > Thanks, > > Fairoz > > > >> -Original Message- > >> From: Vladimir Kozlov > >> Sent: Monday, August 17, 2020 11:22 PM > >> To: Fairoz Matte ; hotspot-compiler- > >> d...@openjdk.java.net; serviceability-dev@openjdk.java.net > >> Cc: Coleen Phillimore ; Dean Long > >> > >> Subject: Re: RFR(s): 8248295: > >> serviceability/jvmti/CompiledMethodLoad/Zombie.java failure with > >> Graal > >> > >> Hi Fairoz, > >> > >> How you determine that +10Mb is enough with Graal? > >> > >> Thanks, > >> Vladimir > >> > >> On 8/17/20 5:46 AM, Fairoz Matte wrote: > >>> Hi, > >>> > >>> > >>> > >>> Please review this small test change to work with Graal. > >>> > >>> > >>> > >>> Background: > >>> > >>> Graal require more code cache compared to c1/c2. but the test case > >>> always > >> set it to 20MB. This may not be sufficient when running graal. > >>> > >>> Default configuration for ReservedCodeCacheSize = 250MB > >>> > >>> With graal enabled, ReservedCodeCacheSize = 350MB > >>> > >>> > >>> > >>> Either we can modify the framework to honor ReservedCodeCacheSize > >>> for > >> graal or just update the testcase. > >>> > >>> There are not many test cases they rely on ReservedCodeCacheSize or > >> InitialCodeCacheSize. So the fix prefer the later one. > >>> > >>> > >>> > >>> JBS - https://bugs.openjdk.java.net/browse/JDK-8248295 > >>> > >>> Webrev - http://cr.openjdk.java.net/~fmatte/8248295/webrev.00/ > >>> > >>> > >>> > >>> Thanks, > >>> > >>> Fairoz > >>> > >>> > >>>
RE: RFR(s): 8248295: serviceability/jvmti/CompiledMethodLoad/Zombie.java failure with Graal
Hi Vladimir, Thanks for looking into. This is intermittent crash, and is reproducible in windows debug build environment. Below is the testing performed. 1. Issues observed 7/100 runs, ReservedCodeCacheSize=20m with "-XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI -XX:+UseJVMCICompiler" 2. Issues observed 0/300 runs, ReservedCodeCacheSize=30m with "-XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI -XX:+UseJVMCICompiler" Thanks, Fairoz > -Original Message- > From: Vladimir Kozlov > Sent: Monday, August 17, 2020 11:22 PM > To: Fairoz Matte ; hotspot-compiler- > d...@openjdk.java.net; serviceability-dev@openjdk.java.net > Cc: Coleen Phillimore ; Dean Long > > Subject: Re: RFR(s): 8248295: > serviceability/jvmti/CompiledMethodLoad/Zombie.java failure with Graal > > Hi Fairoz, > > How you determine that +10Mb is enough with Graal? > > Thanks, > Vladimir > > On 8/17/20 5:46 AM, Fairoz Matte wrote: > > Hi, > > > > > > > > Please review this small test change to work with Graal. > > > > > > > > Background: > > > > Graal require more code cache compared to c1/c2. but the test case always > set it to 20MB. This may not be sufficient when running graal. > > > > Default configuration for ReservedCodeCacheSize = 250MB > > > > With graal enabled, ReservedCodeCacheSize = 350MB > > > > > > > > Either we can modify the framework to honor ReservedCodeCacheSize for > graal or just update the testcase. > > > > There are not many test cases they rely on ReservedCodeCacheSize or > InitialCodeCacheSize. So the fix prefer the later one. > > > > > > > > JBS - https://bugs.openjdk.java.net/browse/JDK-8248295 > > > > Webrev - http://cr.openjdk.java.net/~fmatte/8248295/webrev.00/ > > > > > > > > Thanks, > > > > Fairoz > > > > > >
RFR(s): 8248295: serviceability/jvmti/CompiledMethodLoad/Zombie.java failure with Graal
Hi, Please review this small test change to work with Graal. Background: Graal require more code cache compared to c1/c2. but the test case always set it to 20MB. This may not be sufficient when running graal. Default configuration for ReservedCodeCacheSize = 250MB With graal enabled, ReservedCodeCacheSize = 350MB Either we can modify the framework to honor ReservedCodeCacheSize for graal or just update the testcase. There are not many test cases they rely on ReservedCodeCacheSize or InitialCodeCacheSize. So the fix prefer the later one. JBS - https://bugs.openjdk.java.net/browse/JDK-8248295 Webrev - http://cr.openjdk.java.net/~fmatte/8248295/webrev.00/ Thanks, Fairoz
RE: RFR(s): 8236042: [TESTBUG] serviceability/sa/ClhsdbCDSCore.java fails with -Xcomp -XX:TieredStopAtLevel=1
Thanks Vladimir for the review. Thanks for mentioning the reasons for MDO's not being generated, I have added them as comment in bug for future reference. Thanks, Fairoz > -Original Message- > From: Vladimir Kozlov > Sent: Saturday, July 11, 2020 5:36 AM > To: Fairoz Matte ; Chris Plummer > ; hotspot-compiler-...@openjdk.java.net; > serviceability-dev@openjdk.java.net > Subject: Re: RFR(s): 8236042: [TESTBUG] serviceability/sa/ClhsdbCDSCore.java > fails with -Xcomp -XX:TieredStopAtLevel=1 > > Fix is good. > > I think next are reasons you don't get MDO in this scenario. > > Tier1 (C1 compilation) does not generate profiling code and does not created > MDO. C1 request MDO only with tiers 2 and 3 [1][2]. > > With -Xcomp flag a Java method is not executed in Interpreter but requests its > compilation and waits when it is finished. As result MDO is not created in > Interpreter too. May be late if a method is deoptimized it will be executed in > Interpreter and MDO will be created. > > Thanks, > Vladimir > > [1] > http://hg.openjdk.java.net/jdk/jdk/file/796c9fa50850/src/hotspot/share/c1/c1_ > Compilation.hpp#l226 > [2] > http://hg.openjdk.java.net/jdk/jdk/file/796c9fa50850/src/hotspot/share/c1/c1_ > Compilation.cpp#l381 > > On 7/7/20 8:47 PM, Fairoz Matte wrote: > > Thanks Chris, for the review comments. > > > > I have updated the suggested change. > > > > Thanks, > > Fairoz > > > >> -Original Message- > >> From: Chris Plummer > >> Sent: Wednesday, July 8, 2020 3:38 AM > >> To: Fairoz Matte ; hotspot-compiler- > >> d...@openjdk.java.net; serviceability-dev@openjdk.java.net > >> Subject: Re: RFR(s): 8236042: [TESTBUG] > >> serviceability/sa/ClhsdbCDSCore.java > >> fails with -Xcomp -XX:TieredStopAtLevel=1 > >> > >> Hi Fairoz, > >> > >> Looks good, except for the missing space in "if(testJavaOpts...". > >> > >> thanks, > >> > >> Chris > >> > >> On 7/7/20 7:49 AM, Fairoz Matte wrote: > >>> Hi, > >>> > >>> Please review this small test change to consider the scenario when > >>> there is no > >> "printmdo" output > >>> > >>> JBS - https://bugs.openjdk.java.net/browse/JDK-8236042 > >>> Webrev - http://cr.openjdk.java.net/~fmatte/8236042/webrev.00/ > >>> > >>> Thanks, > >>> Fairoz > >>
RE: RFR(s): 8236042: [TESTBUG] serviceability/sa/ClhsdbCDSCore.java fails with -Xcomp -XX:TieredStopAtLevel=1
Thanks Chris, for the review comments. I have updated the suggested change. Thanks, Fairoz > -Original Message- > From: Chris Plummer > Sent: Wednesday, July 8, 2020 3:38 AM > To: Fairoz Matte ; hotspot-compiler- > d...@openjdk.java.net; serviceability-dev@openjdk.java.net > Subject: Re: RFR(s): 8236042: [TESTBUG] serviceability/sa/ClhsdbCDSCore.java > fails with -Xcomp -XX:TieredStopAtLevel=1 > > Hi Fairoz, > > Looks good, except for the missing space in "if(testJavaOpts...". > > thanks, > > Chris > > On 7/7/20 7:49 AM, Fairoz Matte wrote: > > Hi, > > > > Please review this small test change to consider the scenario when there is > > no > "printmdo" output > > > > JBS - https://bugs.openjdk.java.net/browse/JDK-8236042 > > Webrev - http://cr.openjdk.java.net/~fmatte/8236042/webrev.00/ > > > > Thanks, > > Fairoz >
RFR(s): 8236042: [TESTBUG] serviceability/sa/ClhsdbCDSCore.java fails with -Xcomp -XX:TieredStopAtLevel=1
Hi, Please review this small test change to consider the scenario when there is no "printmdo" output JBS - https://bugs.openjdk.java.net/browse/JDK-8236042 Webrev - http://cr.openjdk.java.net/~fmatte/8236042/webrev.00/ Thanks, Fairoz
RE: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect and corresponsing logic seems to be broken
Thanks Serguei, Leonid for the reviews. Thanks, Fairoz > -Original Message- > From: Leonid Mesnik > Sent: Wednesday, June 10, 2020 10:19 PM > To: Fairoz Matte > Cc: Serguei Spitsyn ; Erik Gahlin > ; serviceability-dev@openjdk.java.net > Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is > incorrect > and corresponsing logic seems to be broken > > Looks good, no other webrev is needed. > > Leonid > > On 6/10/20 12:28 AM, serguei.spit...@oracle.com wrote: > > > > On 6/9/20 23:35, Fairoz Matte wrote: > >> Hi Serguei, > >> > >> Thanks for the clarification. > >> I will work on to move isJFRActive () method from the > >> TestDebuggerType2 to HeapWalkingDebugger > > > > Probably, there is no need in another webrev if you move it. > > But you did not get a final thumbs up from Leonid yet. > > > > Thanks, > > Serguei > > > >> Thanks, > >> Fairoz > >> > >>> -Original Message- > >>> From: Serguei Spitsyn > >>> Sent: Wednesday, June 10, 2020 11:42 AM > >>> To: Fairoz Matte ; Leonid Mesnik > >>> ; Erik Gahlin > >>> Cc: serviceability-dev@openjdk.java.net > >>> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() > >>> is incorrect > >>> and corresponsing logic seems to be broken > >>> > >>> Hi Fairoz, > >>> > >>> It is confusing there is methods with the same name isJFRActive on > >>> both debuggee and debugger side. > >>> Leonid is talking about the isJFRActive that belongs to the debugger. > >>> He suggests to move this method from the TestDebuggerType2 to > >>> HeapWalkingDebugger. > >>> The reason is the HeapWalkingDebugger should have a knowledge about > >>> the HeapWalkingDebuggee, not its super class TestDebuggerType2. > >>> It looks like a good suggestion to me. > >>> > >>> Thanks, > >>> Serguei > >>> > >>> > >>> On 6/9/20 23:00, Fairoz Matte wrote: > >>>> Hi Leonid, > >>>> > >>>> The call isJFRActive() need to be executed on HeapwalkingDebuggee > >>>> side. > >>>> This is what my understanding is. > >>>> > >>>> Thanks, > >>>> Fairoz > >>>> > >>>>> -Original Message- > >>>>> From: Leonid Mesnik > >>>>> Sent: Wednesday, June 10, 2020 1:16 AM > >>>>> To: Serguei Spitsyn ; Fairoz Matte > >>>>> ; Erik Gahlin > >>>>> Cc: serviceability-dev@openjdk.java.net > >>>>> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() > >>>>> is incorrect and corresponsing logic seems to be broken > >>>>> > >>>>> http://cr.openjdk.java.net/~fmatte/8243451/webrev.09/test/hotspot/ > >>>>> jtr eg/vmT estbase/nsk/share/jdi/TestDebuggerType2.java.udiff.html > >>>>> > >>>>> I see that isJFRActive() depends on > >>>>> "nsk.share.jdi.HeapwalkingDebuggee". > >>>>> It is not going to work of debugee is not > >>> "nsk.share.jdi.HeapwalkingDebuggee". > >>>>> Shouldn't it be placed in HeapWalkingDebugger? > >>>>> > >>>>> Leonid > >>>>> > >>>>> On 6/8/20 9:26 PM, serguei.spit...@oracle.com wrote: > >>>>>> Hi Fairoz, > >>>>>> > >>>>>> LGTM. > >>>>>> > >>>>>> Thanks, > >>>>>> Serguei > >>>>>> > >>>>>> > >>>>>> On 6/8/20 21:20, Fairoz Matte wrote: > >>>>>>> Hi Serguei, > >>>>>>> > >>>>>>> Thanks for the clarifications, > >>>>>>> I have incorporated the 2nd suggestion, below is the webrev, > >>>>>>> http://cr.openjdk.java.net/~fmatte/8243451/webrev.09/ > >>>>>>> > >>>>>>> Thanks, > >>>>>>> Fairoz > >>>>>>> > >>>>>>> From: Serguei Spitsyn > >>>>>>> Sent: Monday, June 8, 2020 10:34 PM > >>>>>>> To: Fairoz Matte ; Erik Gahlin > >>>>>>> > >>>>&g
RE: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect and corresponsing logic seems to be broken
Hi Serguei,
Thanks for the clarification.
I will work on to move isJFRActive () method from the TestDebuggerType2 to
HeapWalkingDebugger
Thanks,
Fairoz
> -Original Message-
> From: Serguei Spitsyn
> Sent: Wednesday, June 10, 2020 11:42 AM
> To: Fairoz Matte ; Leonid Mesnik
> ; Erik Gahlin
> Cc: serviceability-dev@openjdk.java.net
> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is
> incorrect
> and corresponsing logic seems to be broken
>
> Hi Fairoz,
>
> It is confusing there is methods with the same name isJFRActive on both
> debuggee and debugger side.
> Leonid is talking about the isJFRActive that belongs to the debugger.
> He suggests to move this method from the TestDebuggerType2 to
> HeapWalkingDebugger.
> The reason is the HeapWalkingDebugger should have a knowledge about the
> HeapWalkingDebuggee, not its super class TestDebuggerType2.
> It looks like a good suggestion to me.
>
> Thanks,
> Serguei
>
>
> On 6/9/20 23:00, Fairoz Matte wrote:
> > Hi Leonid,
> >
> > The call isJFRActive() need to be executed on HeapwalkingDebuggee side.
> > This is what my understanding is.
> >
> > Thanks,
> > Fairoz
> >
> >> -Original Message-
> >> From: Leonid Mesnik
> >> Sent: Wednesday, June 10, 2020 1:16 AM
> >> To: Serguei Spitsyn ; Fairoz Matte
> >> ; Erik Gahlin
> >> Cc: serviceability-dev@openjdk.java.net
> >> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is
> >> incorrect and corresponsing logic seems to be broken
> >>
> >> http://cr.openjdk.java.net/~fmatte/8243451/webrev.09/test/hotspot/jtr
> >> eg/vmT estbase/nsk/share/jdi/TestDebuggerType2.java.udiff.html
> >>
> >> I see that isJFRActive() depends on "nsk.share.jdi.HeapwalkingDebuggee".
> >> It is not going to work of debugee is not
> "nsk.share.jdi.HeapwalkingDebuggee".
> >>
> >> Shouldn't it be placed in HeapWalkingDebugger?
> >>
> >> Leonid
> >>
> >> On 6/8/20 9:26 PM, serguei.spit...@oracle.com wrote:
> >>> Hi Fairoz,
> >>>
> >>> LGTM.
> >>>
> >>> Thanks,
> >>> Serguei
> >>>
> >>>
> >>> On 6/8/20 21:20, Fairoz Matte wrote:
> >>>> Hi Serguei,
> >>>>
> >>>> Thanks for the clarifications,
> >>>> I have incorporated the 2nd suggestion, below is the webrev,
> >>>> http://cr.openjdk.java.net/~fmatte/8243451/webrev.09/
> >>>>
> >>>> Thanks,
> >>>> Fairoz
> >>>>
> >>>> From: Serguei Spitsyn
> >>>> Sent: Monday, June 8, 2020 10:34 PM
> >>>> To: Fairoz Matte ; Erik Gahlin
> >>>>
> >>>> Cc: serviceability-dev@openjdk.java.net
> >>>> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active()
> >>>> is incorrect and corresponsing logic seems to be broken
> >>>>
> >>>> Hi Fairoz,
> >>>>
> >>>>
> >>>> On 6/8/20 02:08, mailto:serguei.spit...@oracle.com wrote:
> >>>> Hi Fairoz,
> >>>>
> >>>> There are two different isJFRActive() methods, one is on debuggee
> >>>> side and another on the debugger side.
> >>>> The one on debuggee side is better to keep in Debuggee.java (where
> >>>> it was before) instead of moving it to HeapwalkingDebuggee.java.
> >>>> It is okay to keep the call to it in the HeapwalkingDebuggee.java.
> >>>>
> >>>> Please, skip this suggestion as Debugger.java is not one of supers
> >>>> of HeapwalkingDebuggee.java as I've assumed.
> >>>>
> >>>> Thanks,
> >>>> Serguei
> >>>>
> >>>>
> >>>> + protected boolean isJFRActive() {
> >>>> + boolean isJFRActive = false;
> >>>> + ReferenceType referenceType =
> >>>> debuggee.classByName("nsk.share.jdi.HeapwalkingDebuggee");
> >>>> + if (referenceType == null)
> >>>> + throw new RuntimeException("Debugeee is not initialized
> >>>> yet");
> >>>> +
> >>>> + Field isJFRActiveFld =
> >>>> referenceType.fieldByName("isJFRActive");
> >>>> + isJFRActive =
> >>>> ((BooleanValue)referenceType.getValue(isJFRActiveFld)).value();
&
RE: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect and corresponsing logic seems to be broken
Hi Leonid,
The call isJFRActive() need to be executed on HeapwalkingDebuggee side.
This is what my understanding is.
Thanks,
Fairoz
> -Original Message-
> From: Leonid Mesnik
> Sent: Wednesday, June 10, 2020 1:16 AM
> To: Serguei Spitsyn ; Fairoz Matte
> ; Erik Gahlin
> Cc: serviceability-dev@openjdk.java.net
> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is
> incorrect
> and corresponsing logic seems to be broken
>
> http://cr.openjdk.java.net/~fmatte/8243451/webrev.09/test/hotspot/jtreg/vmT
> estbase/nsk/share/jdi/TestDebuggerType2.java.udiff.html
>
> I see that isJFRActive() depends on "nsk.share.jdi.HeapwalkingDebuggee".
> It is not going to work of debugee is not "nsk.share.jdi.HeapwalkingDebuggee".
>
> Shouldn't it be placed in HeapWalkingDebugger?
>
> Leonid
>
> On 6/8/20 9:26 PM, serguei.spit...@oracle.com wrote:
> > Hi Fairoz,
> >
> > LGTM.
> >
> > Thanks,
> > Serguei
> >
> >
> > On 6/8/20 21:20, Fairoz Matte wrote:
> >> Hi Serguei,
> >>
> >> Thanks for the clarifications,
> >> I have incorporated the 2nd suggestion, below is the webrev,
> >> http://cr.openjdk.java.net/~fmatte/8243451/webrev.09/
> >>
> >> Thanks,
> >> Fairoz
> >>
> >> From: Serguei Spitsyn
> >> Sent: Monday, June 8, 2020 10:34 PM
> >> To: Fairoz Matte ; Erik Gahlin
> >>
> >> Cc: serviceability-dev@openjdk.java.net
> >> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is
> >> incorrect and corresponsing logic seems to be broken
> >>
> >> Hi Fairoz,
> >>
> >>
> >> On 6/8/20 02:08, mailto:serguei.spit...@oracle.com wrote:
> >> Hi Fairoz,
> >>
> >> There are two different isJFRActive() methods, one is on debuggee
> >> side and another on the debugger side.
> >> The one on debuggee side is better to keep in Debuggee.java (where it
> >> was before) instead of moving it to HeapwalkingDebuggee.java.
> >> It is okay to keep the call to it in the HeapwalkingDebuggee.java.
> >>
> >> Please, skip this suggestion as Debugger.java is not one of supers of
> >> HeapwalkingDebuggee.java as I've assumed.
> >>
> >> Thanks,
> >> Serguei
> >>
> >>
> >> + protected boolean isJFRActive() {
> >> + boolean isJFRActive = false;
> >> + ReferenceType referenceType =
> >> debuggee.classByName("nsk.share.jdi.HeapwalkingDebuggee");
> >> + if (referenceType == null)
> >> + throw new RuntimeException("Debugeee is not initialized
> >> yet");
> >> +
> >> + Field isJFRActiveFld =
> >> referenceType.fieldByName("isJFRActive");
> >> + isJFRActive =
> >> ((BooleanValue)referenceType.getValue(isJFRActiveFld)).value();
> >> + return isJFRActive;
> >> }
> >> It is better to remove the line:
> >> + boolean isJFRActive = false;
> >> and just change this one:
> >> + boolean isJFRActive =
> >> ((BooleanValue)referenceType.getValue(isJFRActiveFld)).value();
> >>
> >> Otherwise, it looks good to me.
> >> I hope, it really works now.
> >>
> >> Thanks,
> >> Serguei
> >>
> >> On 6/8/20 00:26, Fairoz Matte wrote:
> >> Hi Serguei, Erik,
> >> Thanks for the reviews,
> >> Below webrev contains the suggested changes,
> >> http://cr.openjdk.java.net/~fmatte/8243451/webrev.08/
> >> The only thing I couldn’t do is to keep the local copy of
> >> isJFRActive() in HeapwalkingDebugger, The method is called in debugee
> >> code.
> >> In debugger, we have access to debugee before test started or after
> >> test completes.
> >> isJFRActive() method need to be executed during the test execution.
> >> Hence I didn’t find place to initialize and cannot make local copy.
> >> Thanks,
> >> Fairoz
> >> From: Serguei Spitsyn
> >> Sent: Tuesday, June 2, 2020 7:57 AM
> >> To: Fairoz Matte mailto:fairoz.ma...@oracle.com; Erik Gahlin
> >> mailto:erik.gah...@oracle.com
> >> Cc: mailto:serviceability-dev@openjdk.java.net
> >> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is
> >> incorrect and corresponsing logic seems to be broken
> >> On 6/1/20 12:30, mailto:serguei.spit...@oracle.com wrote:
> >> Hi Fairoz,
> >>
> &
RE: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect and corresponsing logic seems to be broken
Hi Serguei,
Thanks for the clarifications,
I have incorporated the 2nd suggestion, below is the webrev,
http://cr.openjdk.java.net/~fmatte/8243451/webrev.09/
Thanks,
Fairoz
From: Serguei Spitsyn
Sent: Monday, June 8, 2020 10:34 PM
To: Fairoz Matte ; Erik Gahlin
Cc: serviceability-dev@openjdk.java.net
Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect
and corresponsing logic seems to be broken
Hi Fairoz,
On 6/8/20 02:08, mailto:serguei.spit...@oracle.com wrote:
Hi Fairoz,
There are two different isJFRActive() methods, one is on debuggee side and
another on the debugger side.
The one on debuggee side is better to keep in Debuggee.java (where it was
before) instead of moving it to HeapwalkingDebuggee.java.
It is okay to keep the call to it in the HeapwalkingDebuggee.java.
Please, skip this suggestion as Debugger.java is not one of supers of
HeapwalkingDebuggee.java as I've assumed.
Thanks,
Serguei
+protected boolean isJFRActive() {
+boolean isJFRActive = false;
+ReferenceType referenceType =
debuggee.classByName("nsk.share.jdi.HeapwalkingDebuggee");
+if (referenceType == null)
+ throw new RuntimeException("Debugeee is not initialized yet");
+
+Field isJFRActiveFld = referenceType.fieldByName("isJFRActive");
+isJFRActive =
((BooleanValue)referenceType.getValue(isJFRActiveFld)).value();
+return isJFRActive;
}
It is better to remove the line:
+boolean isJFRActive = false;
and just change this one:
+boolean isJFRActive =
((BooleanValue)referenceType.getValue(isJFRActiveFld)).value();
Otherwise, it looks good to me.
I hope, it really works now.
Thanks,
Serguei
On 6/8/20 00:26, Fairoz Matte wrote:
Hi Serguei, Erik,
Thanks for the reviews,
Below webrev contains the suggested changes,
http://cr.openjdk.java.net/~fmatte/8243451/webrev.08/
The only thing I couldn’t do is to keep the local copy of isJFRActive() in
HeapwalkingDebugger,
The method is called in debugee code.
In debugger, we have access to debugee before test started or after test
completes.
isJFRActive() method need to be executed during the test execution. Hence I
didn’t find place to initialize and cannot make local copy.
Thanks,
Fairoz
From: Serguei Spitsyn
Sent: Tuesday, June 2, 2020 7:57 AM
To: Fairoz Matte mailto:fairoz.ma...@oracle.com; Erik Gahlin
mailto:erik.gah...@oracle.com
Cc: mailto:serviceability-dev@openjdk.java.net
Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect
and corresponsing logic seems to be broken
On 6/1/20 12:30, mailto:serguei.spit...@oracle.com wrote:
Hi Fairoz,
It looks okay in general.
But I'm not sure this check is going to work.
The problem is the HeapwalkingDebuggee.useStrictCheck method is invoked in the
context of the HeapwalkingDebugger process, not the HeapwalkingDebuggee process.
Probably, you wanted to get this bit of information from the Debuggee process.
The debuggee has to evaluate it itself and store in some field.
The debugger should use the JDI to get this value from the debuggee.
Thanks,
Serguei
I'm not sure, what exactly you wanted to do here.
It can occasionally work for you as long as both processes are run with the
same options.
Thanks,
Serguei
On 6/1/20 08:52, Fairoz Matte wrote:
Hi Erik,
Thanks for the review, below is the updated webrev.
http://cr.openjdk.java.net/~fmatte/8243451/webrev.02/
Thanks,
Fairoz
-Original Message-
From: Erik Gahlin
Sent: Monday, June 1, 2020 4:26 PM
To: Fairoz Matte mailto:fairoz.ma...@oracle.com
Cc: mailto:serviceability-dev@openjdk.java.net
Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect
and corresponsing logic seems to be broken
Hi Fairoz,
What I think you need to do is something like this:
if (className.equals("java.lang.Thread")) {
return !isJfrInitialized();
}
...
private static boolean isJfrInitialized() {
try {
Class clazz = Class.forName("jdk.jfr.FlightRecorder");
Method method = clazz.getDeclaredMethod("isInitialized",
new Class[0]);
return (boolean) method.invoke(null, new Object[0]);
} catch (Exception e) {
return false;
}
}
Erik
On 2020-06-01 12:30, Fairoz Matte wrote:
Hi Erik,
Thanks for your quick response,
Below is the updated webrev to handle if jfr module is not present
http://cr.openjdk.java.net/~fmatte/8243451/webrev.01/
Thanks,
Fairoz
-Original Message-----
From: Erik Gahlin
Sent: Monday, June 1, 2020 2:31 PM
To: Fairoz Matte mailto:fairoz.ma...@oracle.com
Cc: mailto:serviceability-dev@openjdk.java.net
Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is
incorrect and corresponsing logic seems to be broken
Hi Fairoz,
If the test needs to run with builds where the JFR module is not
present(?), you
RE: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect and corresponsing logic seems to be broken
Hi Serguei, Erik,
Thanks for the reviews,
Below webrev contains the suggested changes,
http://cr.openjdk.java.net/~fmatte/8243451/webrev.08/
The only thing I couldn’t do is to keep the local copy of isJFRActive() in
HeapwalkingDebugger,
The method is called in debugee code.
In debugger, we have access to debugee before test started or after test
completes.
isJFRActive() method need to be executed during the test execution. Hence I
didn’t find place to initialize and cannot make local copy.
Thanks,
Fairoz
From: Serguei Spitsyn
Sent: Tuesday, June 2, 2020 7:57 AM
To: Fairoz Matte ; Erik Gahlin
Cc: serviceability-dev@openjdk.java.net
Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect
and corresponsing logic seems to be broken
On 6/1/20 12:30, HYPERLINK
"mailto:serguei.spit...@oracle.com"serguei.spit...@oracle.com wrote:
Hi Fairoz,
It looks okay in general.
But I'm not sure this check is going to work.
The problem is the HeapwalkingDebuggee.useStrictCheck method is invoked in the
context of the HeapwalkingDebugger process, not the HeapwalkingDebuggee process.
Probably, you wanted to get this bit of information from the Debuggee process.
The debuggee has to evaluate it itself and store in some field.
The debugger should use the JDI to get this value from the debuggee.
Thanks,
Serguei
I'm not sure, what exactly you wanted to do here.
It can occasionally work for you as long as both processes are run with the
same options.
Thanks,
Serguei
On 6/1/20 08:52, Fairoz Matte wrote:
Hi Erik,
Thanks for the review, below is the updated webrev.
http://cr.openjdk.java.net/~fmatte/8243451/webrev.02/
Thanks,
Fairoz
-Original Message-
From: Erik Gahlin
Sent: Monday, June 1, 2020 4:26 PM
To: Fairoz Matte HYPERLINK
"mailto:fairoz.ma...@oracle.com;
Cc: HYPERLINK
"mailto:serviceability-dev@openjdk.java.net"serviceability-dev@openjdk.java.net
Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect
and corresponsing logic seems to be broken
Hi Fairoz,
What I think you need to do is something like this:
if (className.equals("java.lang.Thread")) {
return !isJfrInitialized();
}
...
private static boolean isJfrInitialized() {
try {
Class clazz = Class.forName("jdk.jfr.FlightRecorder");
Method method = clazz.getDeclaredMethod("isInitialized",
new Class[0]);
return (boolean) method.invoke(null, new Object[0]);
} catch (Exception e) {
return false;
}
}
Erik
On 2020-06-01 12:30, Fairoz Matte wrote:
Hi Erik,
Thanks for your quick response,
Below is the updated webrev to handle if jfr module is not present
http://cr.openjdk.java.net/~fmatte/8243451/webrev.01/
Thanks,
Fairoz
-Original Message-
From: Erik Gahlin
Sent: Monday, June 1, 2020 2:31 PM
To: Fairoz Matte HYPERLINK
"mailto:fairoz.ma...@oracle.com;
Cc: HYPERLINK
"mailto:serviceability-dev@openjdk.java.net"serviceability-dev@openjdk.java.net
Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is
incorrect and corresponsing logic seems to be broken
Hi Fairoz,
If the test needs to run with builds where the JFR module is not
present(?), you need to do the check using reflection.
If not, looks good.
Erik
On 1 Jun 2020, at 10:27, Fairoz Matte HYPERLINK
"mailto:fairoz.ma...@oracle.com; wrote:
Hi,
Please review this small test infra change to identify at runtime
the JFR is
active or not.
JBS - https://bugs.openjdk.java.net/browse/JDK-8243451
Webrev - http://cr.openjdk.java.net/~fmatte/8243451/webrev.00/
Thanks,
Fairoz
RE: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect and corresponsing logic seems to be broken
Hi Erik,
Thanks for the review, below is the updated webrev.
http://cr.openjdk.java.net/~fmatte/8243451/webrev.02/
Thanks,
Fairoz
> -Original Message-
> From: Erik Gahlin
> Sent: Monday, June 1, 2020 4:26 PM
> To: Fairoz Matte
> Cc: serviceability-dev@openjdk.java.net
> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is
> incorrect
> and corresponsing logic seems to be broken
>
> Hi Fairoz,
>
> What I think you need to do is something like this:
>
> if (className.equals("java.lang.Thread")) {
> return !isJfrInitialized();
> }
>
> ...
>
> private static boolean isJfrInitialized() {
> try {
> Class clazz = Class.forName("jdk.jfr.FlightRecorder");
> Method method = clazz.getDeclaredMethod("isInitialized",
> new Class[0]);
> return (boolean) method.invoke(null, new Object[0]);
> } catch (Exception e) {
> return false;
> }
> }
>
> Erik
>
> On 2020-06-01 12:30, Fairoz Matte wrote:
> > Hi Erik,
> >
> > Thanks for your quick response,
> > Below is the updated webrev to handle if jfr module is not present
> > http://cr.openjdk.java.net/~fmatte/8243451/webrev.01/
> >
> > Thanks,
> > Fairoz
> >
> >> -Original Message-
> >> From: Erik Gahlin
> >> Sent: Monday, June 1, 2020 2:31 PM
> >> To: Fairoz Matte
> >> Cc: serviceability-dev@openjdk.java.net
> >> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is
> >> incorrect and corresponsing logic seems to be broken
> >>
> >> Hi Fairoz,
> >>
> >> If the test needs to run with builds where the JFR module is not
> >> present(?), you need to do the check using reflection.
> >>
> >> If not, looks good.
> >>
> >> Erik
> >>
> >>> On 1 Jun 2020, at 10:27, Fairoz Matte wrote:
> >>>
> >>> Hi,
> >>>
> >>> Please review this small test infra change to identify at runtime
> >>> the JFR is
> >> active or not.
> >>> JBS - https://bugs.openjdk.java.net/browse/JDK-8243451
> >>> Webrev - http://cr.openjdk.java.net/~fmatte/8243451/webrev.00/
> >>>
> >>> Thanks,
> >>> Fairoz
RE: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect and corresponsing logic seems to be broken
Hi Erik, Thanks for your quick response, Below is the updated webrev to handle if jfr module is not present http://cr.openjdk.java.net/~fmatte/8243451/webrev.01/ Thanks, Fairoz > -Original Message- > From: Erik Gahlin > Sent: Monday, June 1, 2020 2:31 PM > To: Fairoz Matte > Cc: serviceability-dev@openjdk.java.net > Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is > incorrect > and corresponsing logic seems to be broken > > Hi Fairoz, > > If the test needs to run with builds where the JFR module is not present(?), > you > need to do the check using reflection. > > If not, looks good. > > Erik > > > On 1 Jun 2020, at 10:27, Fairoz Matte wrote: > > > > Hi, > > > > Please review this small test infra change to identify at runtime the JFR is > active or not. > > > > JBS - https://bugs.openjdk.java.net/browse/JDK-8243451 > > Webrev - http://cr.openjdk.java.net/~fmatte/8243451/webrev.00/ > > > > Thanks, > > Fairoz >
RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect and corresponsing logic seems to be broken
Hi, Please review this small test infra change to identify at runtime the JFR is active or not. JBS - https://bugs.openjdk.java.net/browse/JDK-8243451 Webrev - http://cr.openjdk.java.net/~fmatte/8243451/webrev.00/ Thanks, Fairoz
RE: RFR (S) 8239055: Wrong implementation of VMState.hasListener
Hi Serguei, Thanks for the review. Thanks, Fairoz From: Serguei Spitsyn Sent: Wednesday, February 19, 2020 1:57 AM To: Fairoz Matte ; serviceability-dev@openjdk.java.net Subject: Re: RFR (S) 8239055: Wrong implementation of VMState.hasListener Hi Fairoz, Looks good. Thanks, Serguei On 2/13/20 9:13 PM, Fairoz Matte wrote: Hi, Please review a tiny change to correct the VMState.hasListener implementation. JBS: https://bugs.openjdk.java.net/browse/JDK-8239055 Webrev: http://cr.openjdk.java.net/~fmatte/8239055/webrev.00/ Thanks, Fairoz
RFR (S) 8239055: Wrong implementation of VMState.hasListener
Hi, Please review a tiny change to correct the VMState.hasListener implementation. JBS: https://bugs.openjdk.java.net/browse/JDK-8239055 Webrev: http://cr.openjdk.java.net/~fmatte/8239055/webrev.00/ Thanks, Fairoz
RE: RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work if prelink is enabled
Hi, I have created patches for jdk8u-dev and jdk11u-dev and attached to the bug report. As I am not a committer for both the projects, could some one please sponsor this push? Thanks, Fairoz -Original Message- From: Chris Plummer Sent: Tuesday, December 17, 2019 12:42 AM To: Kevin Walls ; Fairoz Matte ; serviceability-dev@openjdk.java.net Subject: Re: RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work if prelink is enabled +1 On 12/16/19 8:35 AM, Kevin Walls wrote: > Great! 8-) > > On 16/12/2019 15:36, Fairoz Matte wrote: >> Oh yes, >> Thanks Kevin for the review. >> >> Corrected the same - >> http://cr.openjdk.java.net/~fmatte/8235637/webrev.02 >> >> Thanks, >> Fairoz >>
RE: RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work if prelink is enabled
Oh yes, Thanks Kevin for the review. Corrected the same - http://cr.openjdk.java.net/~fmatte/8235637/webrev.02 Thanks, Fairoz -Original Message- From: Kevin Walls Sent: Monday, December 16, 2019 8:44 PM To: Fairoz Matte ; Chris Plummer ; serviceability-dev@openjdk.java.net Subject: Re: RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work if prelink is enabled Nice to know the difference between something that is zero, and something that has failed. 8-) ...oops, that says INAVLID_LOAD_ADDRESS ..not INVALID, so other that the typo yes it looks good. --- Kevin On 13/12/2019 13:02, Fairoz Matte wrote: > Hi Chris, > > Thanks for the review, > > Please find the webrev.01 with usage of INAVLID_LOAD_ADDRESS macro for -1L. > I have also added one more macro for ZERO_LOAD_ADDRESS for 0x0L. > http://cr.openjdk.java.net/~fmatte/8235637/webrev.01/ > > Thanks, > Fairoz > > -Original Message- > From: Chris Plummer > Sent: Thursday, December 12, 2019 9:48 PM > To: Fairoz Matte ; > serviceability-dev@openjdk.java.net > Subject: Re: RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work > if prelink is enabled > > Can you use a macro for -1L? Maybe INAVLID_LOAD_ADDRESS or LOAD_ADDRESS_ERROR. > > Chris > > On 12/11/19 7:10 PM, Fairoz Matte wrote: >> Hi, >> >> Please review this small change, >> Updating error handling, to make sure "lib_base_diff = 0" is still a valid >> scenario even after calc_prelinked_load_address() call. >> >> JBS - https://bugs.openjdk.java.net/browse/JDK-8235637 >> Webrev - http://cr.openjdk.java.net/~fmatte/8235637/webrev.00/ >> >> This patch is provided by Yasumasa Suenaga >> >> Thanks, >> Fairoz
RE: RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work if prelink is enabled
Thanks Chris, > -Original Message- > From: Chris Plummer > Sent: Friday, December 13, 2019 11:09 PM > To: Fairoz Matte ; serviceability- > d...@openjdk.java.net > Subject: Re: RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work if > prelink is enabled > > Looks good. > > thanks, > > Chris > > On 12/13/19 5:02 AM, Fairoz Matte wrote: > > Hi Chris, > > > > Thanks for the review, > > > > Please find the webrev.01 with usage of INAVLID_LOAD_ADDRESS macro > for -1L. > > I have also added one more macro for ZERO_LOAD_ADDRESS for 0x0L. > > http://cr.openjdk.java.net/~fmatte/8235637/webrev.01/ > > > > Thanks, > > Fairoz > > > > -Original Message- > > From: Chris Plummer > > Sent: Thursday, December 12, 2019 9:48 PM > > To: Fairoz Matte ; serviceability- > d...@openjdk.java.net > > Subject: Re: RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work if > prelink is enabled > > > > Can you use a macro for -1L? Maybe INAVLID_LOAD_ADDRESS or > LOAD_ADDRESS_ERROR. > > > > Chris > > > > On 12/11/19 7:10 PM, Fairoz Matte wrote: > >> Hi, > >> > >> Please review this small change, > >> Updating error handling, to make sure "lib_base_diff = 0" is still a valid > scenario even after calc_prelinked_load_address() call. > >> > >> JBS - https://bugs.openjdk.java.net/browse/JDK-8235637 > >> Webrev - http://cr.openjdk.java.net/~fmatte/8235637/webrev.00/ > >> > >> This patch is provided by Yasumasa Suenaga > >> > >> Thanks, > >> Fairoz >
RE: RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work if prelink is enabled
Hi Chris, Thanks for the review, Please find the webrev.01 with usage of INAVLID_LOAD_ADDRESS macro for -1L. I have also added one more macro for ZERO_LOAD_ADDRESS for 0x0L. http://cr.openjdk.java.net/~fmatte/8235637/webrev.01/ Thanks, Fairoz -Original Message- From: Chris Plummer Sent: Thursday, December 12, 2019 9:48 PM To: Fairoz Matte ; serviceability-dev@openjdk.java.net Subject: Re: RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work if prelink is enabled Can you use a macro for -1L? Maybe INAVLID_LOAD_ADDRESS or LOAD_ADDRESS_ERROR. Chris On 12/11/19 7:10 PM, Fairoz Matte wrote: > Hi, > > Please review this small change, > Updating error handling, to make sure "lib_base_diff = 0" is still a valid > scenario even after calc_prelinked_load_address() call. > > JBS - https://bugs.openjdk.java.net/browse/JDK-8235637 > Webrev - http://cr.openjdk.java.net/~fmatte/8235637/webrev.00/ > > This patch is provided by Yasumasa Suenaga > > Thanks, > Fairoz
RE: RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work if prelink is enabled
Hi Yasumasa, Thanks for the review. Sure, I will get them on 8u and 11u. Thanks, Fairoz > -Original Message- > From: Yasumasa Suenaga > Sent: Thursday, December 12, 2019 9:14 AM > To: Fairoz Matte ; serviceability- > d...@openjdk.java.net > Subject: Re: RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work if > prelink is enabled > > Hi Fairoz, > > Looks good! > I want you to backport this change to both jdk11u and 8u. > > > Thanks, > > Yasumasa > > > On 2019/12/12 12:10, Fairoz Matte wrote: > > Hi, > > > > Please review this small change, > > Updating error handling, to make sure "lib_base_diff = 0" is still a valid > scenario even after calc_prelinked_load_address() call. > > > > JBS - https://bugs.openjdk.java.net/browse/JDK-8235637 > > Webrev - http://cr.openjdk.java.net/~fmatte/8235637/webrev.00/ > > > > This patch is provided by Yasumasa Suenaga > > > > Thanks, > > Fairoz > >
RFR: 8235637: jhsdb jmap from OpenJDK 11.0.5 doesn't work if prelink is enabled
Hi, Please review this small change, Updating error handling, to make sure "lib_base_diff = 0" is still a valid scenario even after calc_prelinked_load_address() call. JBS - https://bugs.openjdk.java.net/browse/JDK-8235637 Webrev - http://cr.openjdk.java.net/~fmatte/8235637/webrev.00/ This patch is provided by Yasumasa Suenaga Thanks, Fairoz
RE: RFR (S) 8216352: SA: ClhsdbLauncher should throw errors on Unrecognized commands
Hi Serguei, Thanks for the review. Thanks, Fairoz -Original Message- From: Serguei Spitsyn Sent: Thursday, October 3, 2019 10:26 PM To: Fairoz Matte ; serviceability-dev Subject: Re: RFR (S) 8216352: SA: ClhsdbLauncher should throw errors on Unrecognized commands Hi Fairoz, Looks good. Thanks, Serguei On 10/3/19 06:47, Fairoz Matte wrote: > Hi, > > Please review a tiny change to handle unrecognized command options for > ClhsdbLauncher test. > This patch is already proposed by Gary Adams in comments section. > > JBS: https://bugs.openjdk.java.net/browse/JDK-8216352 > Webrev: http://cr.openjdk.java.net/~fmatte/8216352/webrev.00/ > > Thanks, > Fairoz
RE: RFR (S) 8216352: SA: ClhsdbLauncher should throw errors on Unrecognized commands
Hi Chris, Thanks for the review, do you want me to cover this with regression test? I suspect that it was not required. Thanks, Fairoz -Original Message- From: Chris Plummer Sent: Thursday, October 3, 2019 9:55 PM To: Fairoz Matte ; serviceability-dev Subject: Re: RFR (S) 8216352: SA: ClhsdbLauncher should throw errors on Unrecognized commands Hi Fairoz, I think it looks fine. I guess this implies that we don't have a test that verifies that we get "Unrecognized command." if a bad command is passed, in which case a different approach would be needed (perhaps a flag that says to verify that we get "Unrecognized command." instead of not get it). thanks, Chris On 10/3/19 6:47 AM, Fairoz Matte wrote: > Hi, > > Please review a tiny change to handle unrecognized command options for > ClhsdbLauncher test. > This patch is already proposed by Gary Adams in comments section. > > JBS: https://bugs.openjdk.java.net/browse/JDK-8216352 > Webrev: http://cr.openjdk.java.net/~fmatte/8216352/webrev.00/ > > Thanks, > Fairoz
RFR (S) 8216352: SA: ClhsdbLauncher should throw errors on Unrecognized commands
Hi, Please review a tiny change to handle unrecognized command options for ClhsdbLauncher test. This patch is already proposed by Gary Adams in comments section. JBS: https://bugs.openjdk.java.net/browse/JDK-8216352 Webrev: http://cr.openjdk.java.net/~fmatte/8216352/webrev.00/ Thanks, Fairoz
RE: RFR: 8225715: jhsdb jmap fails to write binary heap dump of a jshell process
Hi Kevin, > -Original Message- > From: Kevin Walls > Sent: Tuesday, July 02, 2019 2:18 PM > To: Fairoz Matte ; serviceability- > d...@openjdk.java.net > Subject: Re: RFR: 8225715: jhsdb jmap fails to write binary heap dump of a > jshell process > > Hi Fairoz - > > Looks good to me too. > > The " -encoding utf8" in the test might be a leftover from something else Yes I will remove that. > (maybe /timeout=240 also?), you could leave it out but no problem either Timeout is required for dump file to be get created. Thanks for the review. Thanks, Fairoz > way. > > Thanks! > Kevin > > > On 01/07/2019 05:31, Fairoz Matte wrote: > > Hi, > > > > Please review a fix that potentially handle NPE and allow heap to dump > > from a process were we get source file name as null. > > > > Background: > > For taking heap dump of JShell process, we get getSourceFileName() as > null. > > Current implementation doesn't have null check. > > This fix will handle the null check and avoid breaking in call to > writeObjectID(sym). > > Regression test provided with patch will demonstrate the problem. > > > > JBS bug - https://bugs.openjdk.java.net/browse/JDK-8225715 > > Webrev - http://cr.openjdk.java.net/~fmatte/8225715/webrev.00/ > > > > Testing: Mach5 tier1,tier2 tier3, hs-tier1, hs-tier2 and hs-tier3 > > tests successfully passed > > > > Thanks, > > Fairoz > >
RFR: 8225715: jhsdb jmap fails to write binary heap dump of a jshell process
Hi, Please review a fix that potentially handle NPE and allow heap to dump from a process were we get source file name as null. Background: For taking heap dump of JShell process, we get getSourceFileName() as null. Current implementation doesn't have null check. This fix will handle the null check and avoid breaking in call to writeObjectID(sym). Regression test provided with patch will demonstrate the problem. JBS bug - https://bugs.openjdk.java.net/browse/JDK-8225715 Webrev - http://cr.openjdk.java.net/~fmatte/8225715/webrev.00/ Testing: Mach5 tier1,tier2 tier3, hs-tier1, hs-tier2 and hs-tier3 tests successfully passed Thanks, Fairoz
RE: [8u-backport] RFR: 8211909: JDWP Transport Listener: dt_socket thread crash
Thanks Jc for the review.. From: JC Beyler Sent: Thursday, October 18, 2018 9:41 PM To: Fairoz Matte Cc: David Holmes ; serviceability-dev@openjdk.java.net Subject: Re: [8u-backport] RFR: 8211909: JDWP Transport Listener: dt_socket thread crash Hi Fairoz, I compared the original and the port, it looks good to me, Jc On Thu, Oct 18, 2018 at 12:55 AM Fairoz Matte mailto:fairoz.ma...@oracle.com"fairoz.ma...@oracle.com> wrote: Thanks David, for the review... > -Original Message- > From: David Holmes > Sent: Thursday, October 18, 2018 1:20 PM > To: Fairoz Matte "mailto:fairoz.ma...@oracle.com"fairoz.ma...@oracle.com>; serviceability- > HYPERLINK "mailto:d...@openjdk.java.net"d...@openjdk.java.net > Subject: Re: [8u-backport] RFR: 8211909: JDWP Transport Listener: dt_socket > thread crash > > Looks good ! Thanks for doing the backport to 8u. > > David > > On 18/10/2018 5:37 PM, Fairoz Matte wrote: > > Hi, > > > > Kindly review the backport of "8211909: JDWP Transport Listener: > dt_socket thread crash" to 8u > > > > code is almost cleanly applied. > > > > Webrev - http://cr.openjdk.java.net/~fmatte/8211909/webrev.00/ > > > > JBS bug - https://bugs.openjdk.java.net/browse/JDK-8211909 > > > > JDK12 changeset - http://hg.openjdk.java.net/jdk/jdk/rev/a4d4c609d70c > > > > Review thread - http://mail.openjdk.java.net/pipermail/serviceability- > dev/2018-October/025515.html > > > > Thanks, > > Fairoz > > -- Thanks, Jc
RE: [8u-backport] RFR: 8211909: JDWP Transport Listener: dt_socket thread crash
Thanks David, for the review... > -Original Message- > From: David Holmes > Sent: Thursday, October 18, 2018 1:20 PM > To: Fairoz Matte ; serviceability- > d...@openjdk.java.net > Subject: Re: [8u-backport] RFR: 8211909: JDWP Transport Listener: dt_socket > thread crash > > Looks good ! Thanks for doing the backport to 8u. > > David > > On 18/10/2018 5:37 PM, Fairoz Matte wrote: > > Hi, > > > > Kindly review the backport of "8211909: JDWP Transport Listener: > dt_socket thread crash" to 8u > > > > code is almost cleanly applied. > > > > Webrev - http://cr.openjdk.java.net/~fmatte/8211909/webrev.00/ > > > > JBS bug - https://bugs.openjdk.java.net/browse/JDK-8211909 > > > > JDK12 changeset - http://hg.openjdk.java.net/jdk/jdk/rev/a4d4c609d70c > > > > Review thread - http://mail.openjdk.java.net/pipermail/serviceability- > dev/2018-October/025515.html > > > > Thanks, > > Fairoz > >
[8u-backport] RFR: 8211909: JDWP Transport Listener: dt_socket thread crash
Hi, Kindly review the backport of "8211909: JDWP Transport Listener: dt_socket thread crash" to 8u code is almost cleanly applied. Webrev - http://cr.openjdk.java.net/~fmatte/8211909/webrev.00/ JBS bug - https://bugs.openjdk.java.net/browse/JDK-8211909 JDK12 changeset - http://hg.openjdk.java.net/jdk/jdk/rev/a4d4c609d70c Review thread - http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-October/025515.html Thanks, Fairoz
RE: [8u-backport] RFR: JDK-8193879: Java debugger hangs on method invocation
Thanks Serguei for the review... > -Original Message- > From: Serguei Spitsyn > Sent: Thursday, October 11, 2018 10:41 PM > To: Fairoz Matte ; serviceability- > d...@openjdk.java.net > Subject: Re: [8u-backport] RFR: JDK-8193879: Java debugger hangs on > method invocation > > Hi Fairoz, > > It looks good to me. > > Thanks, > Serguei > > > On 10/11/18 6:22 AM, Fairoz Matte wrote: > > Hi, > > > > Kindly review the backport of "JDK-8193879: Java debugger hangs on > > method invocation" to 8u Code is almost cleanly applied, test case has been > modified to fit into the JDK8 test framework. > > > > Webrev - http://cr.openjdk.java.net/~fmatte/8193879/webrev.00/ > > > > JBS bug - https://bugs.openjdk.java.net/browse/JDK-8193879 > > > > JDK12 changeset - http://hg.openjdk.java.net/jdk/jdk/rev/218b5b64f102 > > > > Review thread - > > http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-October > > /025365.html > > > > > > Thanks, > > Fairoz >
RE: [8u-backport] RFR: JDK-8193879: Java debugger hangs on method invocation
Hi JC, Thanks for looking into it. I have moved two methods (getTestSourcePath and parseBreakpoints) into Utils (existing helper class) class as part of testlibrary framework. Now it makes sense to have both the methods as public. Updated webrev - http://cr.openjdk.java.net/~fmatte/8193879/webrev.01/ Thanks, Fairoz From: JC Beyler Sent: Thursday, October 11, 2018 9:23 PM To: Fairoz Matte Cc: serviceability-dev@openjdk.java.net Subject: Re: [8u-backport] RFR: JDK-8193879: Java debugger hangs on method invocation Hi Fairoz, The backport looks good to me (not a reviewer though) but I was wondering why you are porting the two methods (getTestSourcePath and parseBreakpoints) into this test? If we port other tests that would require it, would we be doing the same or factorizing the methods into a helper class ? Last nit, those methods could be private no? Thanks, Jc On Thu, Oct 11, 2018 at 6:22 AM Fairoz Matte <mailto:fairoz.ma...@oracle.com> wrote: Hi, Kindly review the backport of "JDK-8193879: Java debugger hangs on method invocation" to 8u Code is almost cleanly applied, test case has been modified to fit into the JDK8 test framework. Webrev - http://cr.openjdk.java.net/~fmatte/8193879/webrev.00/ JBS bug - https://bugs.openjdk.java.net/browse/JDK-8193879 JDK12 changeset - http://hg.openjdk.java.net/jdk/jdk/rev/218b5b64f102 Review thread - http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-October/025365.html Thanks, Fairoz -- Thanks, Jc
[8u-backport] RFR: JDK-8193879: Java debugger hangs on method invocation
Hi, Kindly review the backport of "JDK-8193879: Java debugger hangs on method invocation" to 8u Code is almost cleanly applied, test case has been modified to fit into the JDK8 test framework. Webrev - http://cr.openjdk.java.net/~fmatte/8193879/webrev.00/ JBS bug - https://bugs.openjdk.java.net/browse/JDK-8193879 JDK12 changeset - http://hg.openjdk.java.net/jdk/jdk/rev/218b5b64f102 Review thread - http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-October/025365.html Thanks, Fairoz
RE: [8u-backport] RFR: JDK-8163083: SocketListeningConnector does not allow invocations with port 0
Thanks Daniil... > -Original Message- > From: Daniil Titov > Sent: Thursday, October 04, 2018 9:15 PM > To: Fairoz Matte ; serviceability- > d...@openjdk.java.net > Subject: Re: [8u-backport] RFR: JDK-8163083: SocketListeningConnector does > not allow invocations with port 0 > > Hi Fairoz, > > The backport looks good for me. > > Thanks! > --Daniil > > On 10/3/18, 10:28 PM, "serviceability-dev on behalf of Fairoz Matte" > fairoz.ma...@oracle.com> wrote: > > Hi, > > Kindly review the backport of "JDK-8163083: SocketListeningConnector > does not allow invocations with port 0" to 8u > > Webrev - http://cr.openjdk.java.net/~fmatte/8163083/webrev.00/ > > JBS bug - https://bugs.openjdk.java.net/browse/JDK-8163083 > > JDK12 changeset - http://hg.openjdk.java.net/jdk/jdk/rev/d38cb687d631 > > Review thread - http://mail.openjdk.java.net/pipermail/serviceability- > dev/2018-September/025209.html > > Thanks, > Fairoz > > > >
[8u-backport] RFR: JDK-8163083: SocketListeningConnector does not allow invocations with port 0
Hi, Kindly review the backport of "JDK-8163083: SocketListeningConnector does not allow invocations with port 0" to 8u Webrev - http://cr.openjdk.java.net/~fmatte/8163083/webrev.00/ JBS bug - https://bugs.openjdk.java.net/browse/JDK-8163083 JDK12 changeset - http://hg.openjdk.java.net/jdk/jdk/rev/d38cb687d631 Review thread - http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-September/025209.html Thanks, Fairoz
RE: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on Solaris 12 when loading dumped core
Hi Serguei, Thanks for the review. Thanks, Fairoz > -Original Message- > From: Serguei Spitsyn > Sent: Tuesday, October 02, 2018 11:21 AM > To: Fairoz Matte ; Jini Susan George > ; serviceability-dev@openjdk.java.net > Subject: Re: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on Solaris 12 > when loading dumped core > > > Hi Fairoz, > > It looks good to me. > > Thanks, > Serguei > > > On 9/27/18 10:10 PM, Fairoz Matte wrote: > > Hi Jini, thanks for the review. May I get one more review for this? > > > > Thanks, > > Fairoz > > > >> -Original Message- > >> From: Jini George > >> Sent: Friday, September 28, 2018 10:37 AM > >> To: Fairoz Matte ; serviceability- > >> d...@openjdk.java.net > >> Subject: Re: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on > >> Solaris 12 when loading dumped core > >> > >> This looks good to me, Fairoz. > >> > >> Thanks, > >> Jini. > >> > >> On 9/26/2018 1:37 PM, Fairoz Matte wrote: > >>> Hi Jini, > >>> > >>> Thanks for pointing out that, yes we cannot make that cleanup for JDK8. > >>> Keeping it very simple and taking only changes required to fix > >>> JDK-8164383 http://cr.openjdk.java.net/~fmatte/8164383/webrev.02/ > >>> > >>> I have verified running "test/serviceability/sa/jmap- > >> hashcode/Test8028623.java" test case (found from one of the duplicate > >> issue of JDK-8164383). > >>> Results are as expected before and after the patch on Solaris 12 and > >>> Solaris > >> 10. > >>> Along with that, I have verified with Internal testing and found no > >>> issues > >>> > >>> Thanks, > >>> Fairoz > >>> > >>>> -Original Message- > >>>> From: Jini George > >>>> Sent: Tuesday, September 25, 2018 3:48 PM > >>>> To: Fairoz Matte ; serviceability- > >>>> d...@openjdk.java.net > >>>> Subject: Re: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on > >>>> Solaris 12 when loading dumped core > >>>> > >>>> Hi Fairoz, > >>>> > >>>> I took a better look at the changes and I realized that the cleanup > >>>> related to SOLARIS_11_B159_OR_LATER would be valid for only JDK9 > >>>> and later. Since > >>>> JDK8 is supported for Solaris 10 too, I believe that the cleanup > >>>> related changes done as a part of JDK-8164383 should not be done > >>>> for > >> JDK-8. > >>>> Thanks! > >>>> Jini. > >>>> > >>>> On 9/24/2018 7:21 PM, Fairoz Matte wrote: > >>>>> Hi Jini, > >>>>> > >>>>>> -Original Message- > >>>>>> From: Jini George > >>>>>> Sent: Friday, September 21, 2018 4:07 PM > >>>>>> To: Fairoz Matte ; serviceability- > >>>>>> d...@openjdk.java.net > >>>>>> Subject: Re: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on > >>>>>> Solaris 12 when loading dumped core > >>>>>> > >>>>>> Hi Fairoz, > >>>>>> > >>>>>> This looks good to me. One nit which got missed out in the > >>>>>> original change also is that in saproc.cpp, the following > >>>>>> comments > >>>>>> > >>>>>> 452 > >>>>>> 453 // Pstack_iter() proc_stack_f callback prior to > >>>>>> Nevada-B159 > >>>>>> > >>>>>> 476 // Pstack_iter() proc_stack_f callback in Nevada-B159 or later > >>>>>> 477 /*ARGSUSED*/ > >>>>>> > >>>>> I have incorporated above changes > >>>>> > >>>>>> would not be required anymore. And we would not need the > wrapper > >> to > >>>>>> the callback routine fill_cframe_list() -- as in, we would need > >>>>>> only one routine with the appropriate arguments passed. But you > >>>>>> are free to ignore this since this was not done as a part of the > >>>>>> original > >> change. > >>>>> Removed wrapper_fill_cframe_list function and fill_cframe_list > >>>>> function > >>>> has been used directly. > >>>>> Please find the updated webrev > >>>>> http://cr.openjdk.java.net/~fmatte/8164383/webrev.01/ > >>>>> > >>>>> Thanks, > >>>>> Fairoz > >>>>> > >>>>>> Thanks, > >>>>>> Jini (Not a Reviewer). > >>>>>> > >>>>>> > >>>>>> > >>>>>> On 9/20/2018 7:06 PM, Fairoz Matte wrote: > >>>>>>> Hi, > >>>>>>> > >>>>>>> Kindly review the backport of "JDK-8164383 : jhsdb dumps core on > >>>>>>> Solaris 12 when loading dumped core" to 8u > >>>>>>> > >>>>>>> Webrev - http://cr.openjdk.java.net/~fmatte/8164383/webrev.00/ > >>>>>>> > >>>>>>> JBS bug - https://bugs.openjdk.java.net/browse/JDK-8164383 > >>>>>>> > >>>>>>> JDK9 changeset - > >>>>>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/ce3eaa22b582 > >>>>>>> > >>>>>>> JDK9 review thread - > >>>>>>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-O > >>>>>>> ct > >>>>>>> ob > >>>>>>> er > >>>>>>> /020543.html > >>>>>>> > >>>>>>> Thanks, > >>>>>>> Fairoz > >>>>>>> >
RE: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on Solaris 12 when loading dumped core
Hi Jini, thanks for the review. May I get one more review for this? Thanks, Fairoz > -Original Message- > From: Jini George > Sent: Friday, September 28, 2018 10:37 AM > To: Fairoz Matte ; serviceability- > d...@openjdk.java.net > Subject: Re: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on Solaris 12 > when loading dumped core > > This looks good to me, Fairoz. > > Thanks, > Jini. > > On 9/26/2018 1:37 PM, Fairoz Matte wrote: > > Hi Jini, > > > > Thanks for pointing out that, yes we cannot make that cleanup for JDK8. > > Keeping it very simple and taking only changes required to fix > > JDK-8164383 http://cr.openjdk.java.net/~fmatte/8164383/webrev.02/ > > > > I have verified running "test/serviceability/sa/jmap- > hashcode/Test8028623.java" test case (found from one of the duplicate issue > of JDK-8164383). > > Results are as expected before and after the patch on Solaris 12 and Solaris > 10. > > > > Along with that, I have verified with Internal testing and found no > > issues > > > > Thanks, > > Fairoz > > > >> -Original Message- > >> From: Jini George > >> Sent: Tuesday, September 25, 2018 3:48 PM > >> To: Fairoz Matte ; serviceability- > >> d...@openjdk.java.net > >> Subject: Re: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on > >> Solaris 12 when loading dumped core > >> > >> Hi Fairoz, > >> > >> I took a better look at the changes and I realized that the cleanup > >> related to SOLARIS_11_B159_OR_LATER would be valid for only JDK9 and > >> later. Since > >> JDK8 is supported for Solaris 10 too, I believe that the cleanup > >> related changes done as a part of JDK-8164383 should not be done for > JDK-8. > >> > >> Thanks! > >> Jini. > >> > >> On 9/24/2018 7:21 PM, Fairoz Matte wrote: > >>> Hi Jini, > >>> > >>>> -Original Message- > >>>> From: Jini George > >>>> Sent: Friday, September 21, 2018 4:07 PM > >>>> To: Fairoz Matte ; serviceability- > >>>> d...@openjdk.java.net > >>>> Subject: Re: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on > >>>> Solaris 12 when loading dumped core > >>>> > >>>> Hi Fairoz, > >>>> > >>>> This looks good to me. One nit which got missed out in the original > >>>> change also is that in saproc.cpp, the following comments > >>>> > >>>> 452 > >>>> 453 // Pstack_iter() proc_stack_f callback prior to Nevada-B159 > >>>> > >>>> 476 // Pstack_iter() proc_stack_f callback in Nevada-B159 or later > >>>> 477 /*ARGSUSED*/ > >>>> > >>> > >>> I have incorporated above changes > >>> > >>>> would not be required anymore. And we would not need the wrapper > to > >>>> the callback routine fill_cframe_list() -- as in, we would need > >>>> only one routine with the appropriate arguments passed. But you are > >>>> free to ignore this since this was not done as a part of the original > change. > >>> > >>> Removed wrapper_fill_cframe_list function and fill_cframe_list > >>> function > >> has been used directly. > >>> > >>> Please find the updated webrev > >>> http://cr.openjdk.java.net/~fmatte/8164383/webrev.01/ > >>> > >>> Thanks, > >>> Fairoz > >>> > >>>> > >>>> Thanks, > >>>> Jini (Not a Reviewer). > >>>> > >>>> > >>>> > >>>> On 9/20/2018 7:06 PM, Fairoz Matte wrote: > >>>>> Hi, > >>>>> > >>>>> Kindly review the backport of "JDK-8164383 : jhsdb dumps core on > >>>>> Solaris 12 when loading dumped core" to 8u > >>>>> > >>>>> Webrev - http://cr.openjdk.java.net/~fmatte/8164383/webrev.00/ > >>>>> > >>>>> JBS bug - https://bugs.openjdk.java.net/browse/JDK-8164383 > >>>>> > >>>>> JDK9 changeset - > >>>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/ce3eaa22b582 > >>>>> > >>>>> JDK9 review thread - > >>>>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-Oct > >>>>> ob > >>>>> er > >>>>> /020543.html > >>>>> > >>>>> Thanks, > >>>>> Fairoz > >>>>>
RE: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on Solaris 12 when loading dumped core
Hi Jini, Thanks for pointing out that, yes we cannot make that cleanup for JDK8. Keeping it very simple and taking only changes required to fix JDK-8164383 http://cr.openjdk.java.net/~fmatte/8164383/webrev.02/ I have verified running "test/serviceability/sa/jmap-hashcode/Test8028623.java" test case (found from one of the duplicate issue of JDK-8164383). Results are as expected before and after the patch on Solaris 12 and Solaris 10. Along with that, I have verified with Internal testing and found no issues Thanks, Fairoz > -Original Message- > From: Jini George > Sent: Tuesday, September 25, 2018 3:48 PM > To: Fairoz Matte ; serviceability- > d...@openjdk.java.net > Subject: Re: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on Solaris 12 > when loading dumped core > > Hi Fairoz, > > I took a better look at the changes and I realized that the cleanup related to > SOLARIS_11_B159_OR_LATER would be valid for only JDK9 and later. Since > JDK8 is supported for Solaris 10 too, I believe that the cleanup related > changes done as a part of JDK-8164383 should not be done for JDK-8. > > Thanks! > Jini. > > On 9/24/2018 7:21 PM, Fairoz Matte wrote: > > Hi Jini, > > > >> -Original Message----- > >> From: Jini George > >> Sent: Friday, September 21, 2018 4:07 PM > >> To: Fairoz Matte ; serviceability- > >> d...@openjdk.java.net > >> Subject: Re: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on > >> Solaris 12 when loading dumped core > >> > >> Hi Fairoz, > >> > >> This looks good to me. One nit which got missed out in the original > >> change also is that in saproc.cpp, the following comments > >> > >>452 > >>453 // Pstack_iter() proc_stack_f callback prior to Nevada-B159 > >> > >>476 // Pstack_iter() proc_stack_f callback in Nevada-B159 or later > >>477 /*ARGSUSED*/ > >> > > > > I have incorporated above changes > > > >> would not be required anymore. And we would not need the wrapper to > >> the callback routine fill_cframe_list() -- as in, we would need only > >> one routine with the appropriate arguments passed. But you are free > >> to ignore this since this was not done as a part of the original change. > > > > Removed wrapper_fill_cframe_list function and fill_cframe_list function > has been used directly. > > > > Please find the updated webrev > > http://cr.openjdk.java.net/~fmatte/8164383/webrev.01/ > > > > Thanks, > > Fairoz > > > >> > >> Thanks, > >> Jini (Not a Reviewer). > >> > >> > >> > >> On 9/20/2018 7:06 PM, Fairoz Matte wrote: > >>> Hi, > >>> > >>> Kindly review the backport of "JDK-8164383 : jhsdb dumps core on > >>> Solaris 12 when loading dumped core" to 8u > >>> > >>> Webrev - http://cr.openjdk.java.net/~fmatte/8164383/webrev.00/ > >>> > >>> JBS bug - https://bugs.openjdk.java.net/browse/JDK-8164383 > >>> > >>> JDK9 changeset - > >>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/ce3eaa22b582 > >>> > >>> JDK9 review thread - > >>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-Octob > >>> er > >>> /020543.html > >>> > >>> Thanks, > >>> Fairoz > >>>
RE: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on Solaris 12 when loading dumped core
Hi Jini, > -Original Message- > From: Jini George > Sent: Friday, September 21, 2018 4:07 PM > To: Fairoz Matte ; serviceability- > d...@openjdk.java.net > Subject: Re: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on Solaris 12 > when loading dumped core > > Hi Fairoz, > > This looks good to me. One nit which got missed out in the original change > also is that in saproc.cpp, the following comments > > 452 > 453 // Pstack_iter() proc_stack_f callback prior to Nevada-B159 > > 476 // Pstack_iter() proc_stack_f callback in Nevada-B159 or later > 477 /*ARGSUSED*/ > I have incorporated above changes > would not be required anymore. And we would not need the wrapper to the > callback routine fill_cframe_list() -- as in, we would need only one routine > with the appropriate arguments passed. But you are free to ignore this since > this was not done as a part of the original change. Removed wrapper_fill_cframe_list function and fill_cframe_list function has been used directly. Please find the updated webrev http://cr.openjdk.java.net/~fmatte/8164383/webrev.01/ Thanks, Fairoz > > Thanks, > Jini (Not a Reviewer). > > > > On 9/20/2018 7:06 PM, Fairoz Matte wrote: > > Hi, > > > > Kindly review the backport of "JDK-8164383 : jhsdb dumps core on > > Solaris 12 when loading dumped core" to 8u > > > > Webrev - http://cr.openjdk.java.net/~fmatte/8164383/webrev.00/ > > > > JBS bug - https://bugs.openjdk.java.net/browse/JDK-8164383 > > > > JDK9 changeset - > > http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/ce3eaa22b582 > > > > JDK9 review thread - > > http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-October > > /020543.html > > > > Thanks, > > Fairoz > >
RE: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on Solaris 12 when loading dumped core
Hi Jini, Thanks for the review. Thanks, Fairoz > -Original Message- > From: Jini George > Sent: Friday, September 21, 2018 4:07 PM > To: Fairoz Matte ; serviceability- > d...@openjdk.java.net > Subject: Re: [8u-backport] RFR: JDK-8164383 : jhsdb dumps core on Solaris 12 > when loading dumped core > > Hi Fairoz, > > This looks good to me. One nit which got missed out in the original change > also is that in saproc.cpp, the following comments > > 452 > 453 // Pstack_iter() proc_stack_f callback prior to Nevada-B159 > > 476 // Pstack_iter() proc_stack_f callback in Nevada-B159 or later > 477 /*ARGSUSED*/ > > would not be required anymore. And we would not need the wrapper to the > callback routine fill_cframe_list() -- as in, we would need only one routine > with the appropriate arguments passed. But you are free to ignore this since > this was not done as a part of the original change. > > Thanks, > Jini (Not a Reviewer). > > > > On 9/20/2018 7:06 PM, Fairoz Matte wrote: > > Hi, > > > > Kindly review the backport of "JDK-8164383 : jhsdb dumps core on > > Solaris 12 when loading dumped core" to 8u > > > > Webrev - http://cr.openjdk.java.net/~fmatte/8164383/webrev.00/ > > > > JBS bug - https://bugs.openjdk.java.net/browse/JDK-8164383 > > > > JDK9 changeset - > > http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/ce3eaa22b582 > > > > JDK9 review thread - > > http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-October > > /020543.html > > > > Thanks, > > Fairoz > >
[8u-backport] RFR: JDK-8164383 : jhsdb dumps core on Solaris 12 when loading dumped core
Hi, Kindly review the backport of "JDK-8164383 : jhsdb dumps core on Solaris 12 when loading dumped core" to 8u Webrev - http://cr.openjdk.java.net/~fmatte/8164383/webrev.00/ JBS bug - https://bugs.openjdk.java.net/browse/JDK-8164383 JDK9 changeset - http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/ce3eaa22b582 JDK9 review thread - http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-October/020543.html Thanks, Fairoz
RE: [8u-backport] RFR: JDK-8191948: jdb error: InvalidTypeException: Can't assign double[][][] to double[][][]
Hi Chris and Serguei, Thanks for the review, I will add the appropriate noreg label. Thanks, Fairoz From: Serguei Spitsyn Sent: Wednesday, July 25, 2018 11:24 PM To: Chris Plummer ; Fairoz Matte ; serviceability-dev@openjdk.java.net Subject: Re: [8u-backport] RFR: JDK-8191948: jdb error: InvalidTypeException: Can't assign double[][][] to double[][][] Hi Fairoz, Looks good to me too. Thank you for taking care about this backport! On 7/25/18 10:31, Chris Plummer wrote: Hi Fairoz, The changes look good. I'm not sure what the policy is when part of the (full) backport contains test changes that aren't directly applicable to 8u. You might need some sort of noreg label on the backport CR. The test test/hotspot/jtreg/vmTestbase/nsk/jdb/eval/eval001 is located in the VM testbase which is a separate repository for jdk 8. I agree with Chris, noreg label on the backport CR is probably needed. Thanks, Serguei thanks, Chris On 7/25/18 1:23 AM, Fairoz Matte wrote: Hi, Kindly review the backport of "JDK-8191948: jdb error: InvalidTypeException: Can't assign double[][][] to double[][][]" to 8u Webrev - http://cr.openjdk.java.net/~fmatte/8191948/webrev.00/ JDK 11 bug - https://bugs.openjdk.java.net/browse/JDK-8191948 JDK 11 changeset - http://hg.openjdk.java.net/jdk/jdk11/rev/73c769e0486a Review thread - http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-July/024405.html Thanks, Fairoz
[8u-backport] RFR: JDK-8191948: jdb error: InvalidTypeException: Can't assign double[][][] to double[][][]
Hi, Kindly review the backport of "JDK-8191948: jdb error: InvalidTypeException: Can't assign double[][][] to double[][][]" to 8u Webrev - http://cr.openjdk.java.net/~fmatte/8191948/webrev.00/ JDK 11 bug - https://bugs.openjdk.java.net/browse/JDK-8191948 JDK 11 changeset - http://hg.openjdk.java.net/jdk/jdk11/rev/73c769e0486a Review thread - http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-July/024405.html Thanks, Fairoz
RE: RFR: JDK-8173664: Typo in https://java.net/downloads/heap-snapshot/hprof-binary-format.html
Hi, Please find the updated webrev with suggested changes Webrev - http://cr.openjdk.java.net/~rpatil/8173664/webrev.01/ BugID: https://bugs.openjdk.java.net/browse/JDK-8173664 Thanks, Fairoz > -Original Message- > From: Fairoz Matte > Sent: Thursday, May 11, 2017 9:05 AM > To: David Holmes <david.hol...@oracle.com>; serviceability- > d...@openjdk.java.net > Subject: RE: RFR: JDK-8173664: Typo in https://java.net/downloads/heap- > snapshot/hprof-binary-format.html > > Hi David, > > After having discussion with you over IM things are clear Expected change is - > "There will be a "LOAD CLASS" tag for the type of each array in the dump" > and current changeset missed "the" before type. > > I will make the changes and send it again. > > Thanks, > Fairoz > > > -Original Message- > > From: David Holmes > > Sent: Thursday, May 11, 2017 7:52 AM > > To: Fairoz Matte <fairoz.ma...@oracle.com>; serviceability- > > d...@openjdk.java.net > > Subject: Re: RFR: JDK-8173664: Typo in > > https://java.net/downloads/heap- snapshot/hprof-binary-format.html > > > > On 10/05/2017 9:30 PM, Fairoz Matte wrote: > > > Hi David, > > > > > >> -Original Message- > > >> From: David Holmes > > >> Sent: Wednesday, May 10, 2017 3:26 PM > > >> To: Fairoz Matte <fairoz.ma...@oracle.com>; serviceability- > > >> d...@openjdk.java.net > > >> Subject: Re: RFR: JDK-8173664: Typo in > > >> https://java.net/downloads/heap- snapshot/hprof-binary-format.html > > >> > > >> Hi Fairoz, > > >> > > >> On 10/05/2017 5:53 PM, Fairoz Matte wrote: > > >>> Hi, > > >>> > > >>> Kindly review the small typo fix, applicable only for JDK8 > > >>> BugID: https://bugs.openjdk.java.net/browse/JDK-8173664 > > >>> Webrev: http://cr.openjdk.java.net/~rpatil/8173664/webrev/ > > >> > > >> I think "for type type" was intended to be "for the type". > > >> > > > Yes it does look like after reading multiple times. > > > Thanks for the review I will close as Not an issue > > > > ??? It is still a typo that can be fixed. > > > > David > > > > > Thanks, > > > Fairoz > > > > > >> David > > >> > > >>> Thanks, > > >>> Fairoz > > >>>
RE: RFR: JDK-8173664: Typo in https://java.net/downloads/heap-snapshot/hprof-binary-format.html
Hi David, After having discussion with you over IM things are clear Expected change is - "There will be a "LOAD CLASS" tag for the type of each array in the dump" and current changeset missed "the" before type. I will make the changes and send it again. Thanks, Fairoz > -Original Message- > From: David Holmes > Sent: Thursday, May 11, 2017 7:52 AM > To: Fairoz Matte <fairoz.ma...@oracle.com>; serviceability- > d...@openjdk.java.net > Subject: Re: RFR: JDK-8173664: Typo in https://java.net/downloads/heap- > snapshot/hprof-binary-format.html > > On 10/05/2017 9:30 PM, Fairoz Matte wrote: > > Hi David, > > > >> -Original Message- > >> From: David Holmes > >> Sent: Wednesday, May 10, 2017 3:26 PM > >> To: Fairoz Matte <fairoz.ma...@oracle.com>; serviceability- > >> d...@openjdk.java.net > >> Subject: Re: RFR: JDK-8173664: Typo in > >> https://java.net/downloads/heap- snapshot/hprof-binary-format.html > >> > >> Hi Fairoz, > >> > >> On 10/05/2017 5:53 PM, Fairoz Matte wrote: > >>> Hi, > >>> > >>> Kindly review the small typo fix, applicable only for JDK8 > >>> BugID: https://bugs.openjdk.java.net/browse/JDK-8173664 > >>> Webrev: http://cr.openjdk.java.net/~rpatil/8173664/webrev/ > >> > >> I think "for type type" was intended to be "for the type". > >> > > Yes it does look like after reading multiple times. > > Thanks for the review I will close as Not an issue > > ??? It is still a typo that can be fixed. > > David > > > Thanks, > > Fairoz > > > >> David > >> > >>> Thanks, > >>> Fairoz > >>>
RE: RFR: JDK-8173664: Typo in https://java.net/downloads/heap-snapshot/hprof-binary-format.html
Hi David, > -Original Message- > From: David Holmes > Sent: Wednesday, May 10, 2017 3:26 PM > To: Fairoz Matte <fairoz.ma...@oracle.com>; serviceability- > d...@openjdk.java.net > Subject: Re: RFR: JDK-8173664: Typo in https://java.net/downloads/heap- > snapshot/hprof-binary-format.html > > Hi Fairoz, > > On 10/05/2017 5:53 PM, Fairoz Matte wrote: > > Hi, > > > > Kindly review the small typo fix, applicable only for JDK8 > > BugID: https://bugs.openjdk.java.net/browse/JDK-8173664 > > Webrev: http://cr.openjdk.java.net/~rpatil/8173664/webrev/ > > I think "for type type" was intended to be "for the type". > Yes it does look like after reading multiple times. Thanks for the review I will close as Not an issue Thanks, Fairoz > David > > > Thanks, > > Fairoz > >
RFR: JDK-8173664: Typo in https://java.net/downloads/heap-snapshot/hprof-binary-format.html
Hi, Kindly review the small typo fix, applicable only for JDK8 BugID: https://bugs.openjdk.java.net/browse/JDK-8173664 Webrev: http://cr.openjdk.java.net/~rpatil/8173664/webrev/ Thanks, Fairoz
RE: [7u] RFR: JDK-6626412: jstack using SA prints some info messages into err stream
PING Someone could please review this, It is back port to 7. Only modification is changing info messages from err string - PrintStream err = System.err; + PrintStream out = System.out; And respective err to out in single file Thanks, Fairoz > -Original Message- > From: Fairoz Matte > Sent: Friday, June 17, 2016 3:02 PM > To: serviceability-dev@openjdk.java.net > Subject: [7u] RFR: JDK-6626412: jstack using SA prints some info > messages into err stream > > Hi, > > Please review the backport of bug: "JDK-6626412: jstack using SA prints > some info messages into err stream" > > Webrev: http://cr.openjdk.java.net/~shshahma/6626412/webrev/ > > jdk9 bug: https://bugs.openjdk.java.net/browse/JDK-6626412 > > jdk9 > changeset:http://hg.openjdk.java.net/hsx/hsx25/hotspot/rev/5280822ddfcd > > Review thread: http://mail.openjdk.java.net/pipermail/serviceability- > dev/2013-November/013056.html > > Thanks, > Fairoz
[7u] RFR: JDK-6626412: jstack using SA prints some info messages into err stream
Hi, Please review the backport of bug: "JDK-6626412: jstack using SA prints some info messages into err stream" Webrev: http://cr.openjdk.java.net/~shshahma/6626412/webrev/ jdk9 bug: https://bugs.openjdk.java.net/browse/JDK-6626412 jdk9 changeset:http://hg.openjdk.java.net/hsx/hsx25/hotspot/rev/5280822ddfcd Review thread: http://mail.openjdk.java.net/pipermail/serviceability-dev/2013-November/013056.html Thanks, Fairoz
