Re: RFR: 8209163: SA: Show Object Histogram asserts with ZGC

Thanks Yasumasa and JC for looking at this. I noticed that we seem to do the same thing in ReversePtrsAnalysis.java, i.e. call heapIterationFractionUpdate(0) to get things initialized. So, let's go with webrev.0. cheers, Per On 09/13/2018 07:31 PM, JC Beyler wrote: Looks like webrev.0 is the

RFR: 8210647: libsaproc is being compiled without optimization.

Hi, Could I please get a review of this one-liner fix. It changes optimization of libsaproc from -O0 to -O3 (as per Magnus' suggestion). I've run servicability tests and haven't seen any new failures. Thoughts? Bug: https://bugs.openjdk.java.net/browse/JDK-8210647 webrev: http://cr.openjdk.java.n

Re: RFR: 8210647: libsaproc is being compiled without optimization.

Looks good to me. /Erik On 2018-09-14 06:33, Severin Gehwolf wrote: Hi, Could I please get a review of this one-liner fix. It changes optimization of libsaproc from -O0 to -O3 (as per Magnus' suggestion). I've run servicability tests and haven't seen any new failures. Thoughts? Bug: https://

Re: RFR (L) 8210481: Remove #ifdef cplusplus from vmTestbase

Hi JC, looks good to me. (cc'ing hotspot-dev alias as it affects other teams' tests) -- Igor > On Sep 14, 2018, at 9:50 AM, JC Beyler wrote: > > Hi all, > > Could I get a review for the following webrev: > Webrev: http://cr.openjdk.java.net/~jcbeyler/8210481/webrev.00/ >

Re: RFR (XS) 8210726: Fix up a few minor nits forgotten by JDK-8210665

Hi JC, LGTM. -- Igor > On Sep 14, 2018, at 9:49 AM, JC Beyler wrote: > > Hi all, > > When doing the work for JDK-8210665, I got confused with the webrevs and > forgot to add Serguei's nits for the patch. I apologize for that and here > they are in a separate bug. > > Could I get a review f

Re: RFR (M) 8210700: Clean up JNI_ENV_ARG and factorize the macros for vmTestbase/jvmti/unit tests

Hi Jc, On 09/13/2018 20:05, JC Beyler wrote: Thanks Alexey for the review, I fixed all the " ," issues that the patch changed but there are still at least 29 files that seem to have that issue in the vmTestbase that were not touched by this webrev. I imagine we can do a refactoring in another

Re: RFR (XS) 8210726: Fix up a few minor nits forgotten by JDK-8210665

One minor issue. There's an extra space in the following line:  157 fields[i].fid = env-> GetStaticFieldID(cls, fields[i].name, fields[i].sig); Chris On 9/14/18 9:49 AM, JC Beyler wrote:

Re: RFR (L) 8210481: Remove #ifdef cplusplus from vmTestbase

Looks good. Chris On 9/14/18 10:48 AM, Igor Ignatyev wrote: Hi JC, looks good to me. (cc'ing hotspot-dev alias as it affects other teams' tests) -- Igor

RFR JDK-8210760: [TEST] rewrite com/sun/jdi shell tests to java version - step4

Hi all, please review fix for https://bugs.openjdk.java.net/browse/JDK-8210760 webrev: http://cr.openjdk.java.net/~amenkov/sh2java/step4/webrev.01/ --alex

Re: RFR(M) : 8210732 : remove jdk.testlibrary.Utils

Alan, JC, thank you both for review -- Igor > On Sep 13, 2018, at 11:53 PM, Alan Bateman wrote: > > > > On 14/09/2018 01:07, Igor Ignatyev wrote: >> http://cr.openjdk.java.net/~iignatyev//8210732/webrev.00/index.html >>> 478 lines changed: 3 ins; 319 del; 156 mod; >> Hi all, >> >> could you p

Re: RFR JDK-8210760: [TEST] rewrite com/sun/jdi shell tests to java version - step4

Hi Alex, Just one issue I see. For RedefineTTYLineNumber.java, the original test used to have this comment, which your removed:   52   // line number sensitive!!! Next line must be line 10. It's not clear to me why this test was ever line number sensitive, and whether you removed this sensit

Re: RFR (M) 8210700: Clean up JNI_ENV_ARG and factorize the macros for vmTestbase/jvmti/unit tests

Hi Jc, I looked only at rawmonitor.cpp (I suppose nothing other has been changed). Looks good. --alex On 09/14/2018 13:50, JC Beyler wrote: Hi Alex, Ok I understand now what you mean. I just did a double check on files that had global definitions of jvmtiEnv across the tests (not complete b

RFR (S) 8210775: JVM TI Spec missing copyright

Hi. Please review the following changes to add a copyright line to the end of the generated jvmti.html file: 8210775: JVM TI Spec missing copyright bug: https://bugs.openjdk.java.net/browse/JDK-8210775 webrev: http://cr.openjdk.java.net/~iris/8210775/webrev/ The year ranges are ta

Re: RFR (S) 8210775: JVM TI Spec missing copyright

On 9/14/18 3:01 PM, Iris Clark wrote: Hi. Please review the following changes to add a copyright line to the end of the generated jvmti.html file: 8210775: JVM TI Spec missing copyright bug: https://bugs.openjdk.java.net/browse/JDK-8210775 webrev: http://cr.openjdk.java.net/

Re: RFR JDK-8210760: [TEST] rewrite com/sun/jdi shell tests to java version - step4

Hi Chris, The file history does not contain any info about line number dependency. I'll remove "11 before, 10 afterward" comment. Actually the test is not clear to me. Accordingly the test description jdb report obsolete line number in the case, but the test does not verify its correctness, but

Re: RFR JDK-8210760: [TEST] rewrite com/sun/jdi shell tests to java version - step4

I think checking the output after the second breakpoint would be a good idea. However, rather than checking the line number, maybe just check the contents of the line, which should be included in the breakpoint event output. Chris On 9/14/18 3:23 PM, Alex Menkov wrote: Hi Chris, The file hi

RFR(XS) : 8210779 : 8182404 and 8210732 haven't updated copyright years

http://cr.openjdk.java.net/~iignatyev//8210779/webrev.00/index.html > 36 lines changed: 0 ins; 0 del; 36 mod; Hi all, could you please review this small and trivial follow-up fix for 8182404 and 8210732? I have forgot to update copyright years as a part of these two changesets, this patch fixes

Re: RFR JDK-8210760: [TEST] rewrite com/sun/jdi shell tests to java version - step4

On 09/14/2018 15:03, JC Beyler wrote: Hi Alex, I also saw two nits, in the same file as Chris was mentioning (http://cr.openjdk.java.net/~amenkov/sh2java/step4/webrev.01/test/jdk/com/sun/jdi/RedefineTTYLineNumber.java.udiff.html

Re: RFR JDK-8210760: [TEST] rewrite com/sun/jdi shell tests to java version - step4

Looks like only line numbers are reported correctly, but the content of the line content if not correct :) [jdb] Breakpoint hit: "thread=main", RedefineTTYLineNumberTarg.A(), line=47 bci=0 [jdb] 47System.out.println("in A, about to call B"); [jdb] [jdb] main[1] [jdb] Breakpoint hi

Re: RFR(XS) : 8210779 : 8182404 and 8210732 haven't updated copyright years

Looks like you got them all - reviewed. -Brent On 09/14/2018 04:09 PM, Igor Ignatyev wrote: http://cr.openjdk.java.net/~iignatyev//8210779/webrev.00/index.html 36 lines changed: 0 ins; 0 del; 36 mod; Hi all, could you please review this small and trivial follow-up fix for 8182404 and 821073

Re: RFR (M) 8210700: Clean up JNI_ENV_ARG and factorize the macros for vmTestbase/jvmti/unit tests

Hi Jc, Looks good to me. Thanks, Serguei On 9/13/18 13:26, JC Beyler wrote: Hi all, We have arrived to the last webrev for removing the JNI_ENV macros from

Re: RFR JDK-8210760: [TEST] rewrite com/sun/jdi shell tests to java version - step4

Hmm. I thought that's what the original bug was addressing.   27  * @summary TTY: Need to clear source cache after doing a redefine class Chris On 9/14/18 4:37 PM, Alex Menkov wrote: Looks like only line numbers are reported correctly, but the content of the line content if not correct :)