Re: RFR(S) 8215113: Sampling interval not always correct

if we passed it to the heap sampler. > > Same copyright line comment for HeapMonitor.java, > HeapMonitorArrayAllSampledTest.java and > HeapMonitorStatArrayCorrectnessTest.java > as for memAllocator.cpp. > > Otherwise lgtm, no need for another webrev on my account. > > > Thanks, > > > >

Re: RFR (M) 8212959: Remove booleans from tests in vmTestbase

:-), Jc On Tue, Jan 8, 2019 at 11:14 PM David Holmes wrote: > On 9/01/2019 4:43 pm, JC Beyler wrote: > > Hi David, > > > > I was not planning on doing it but we could. This change came from a > > request to remove the booleans from tests for a prior webrev cha

RFR (M) 8212959: Remove booleans from tests in vmTestbase

Hi all, Fixing up the tests in vmTestbase to not be testing explicitly against NSK_TRUE/NSK_FALSE. Here is the webrev to do that: Bug: https://bugs.openjdk.java.net/browse/JDK-8212959 Webrev: http://cr.openjdk.java.net/~jcbeyler/8212959/webrev.00/ Thanks, Jc

RFR (S) 8212824: Remove unnecessary spaces before/after comparison in vmTestbase

Hi all, Could I get a review for a small-ish webrev that only changes white-spaces (or removes unnecessary new lines): Bug: https://bugs.openjdk.java.net/browse/JDK-8212824 Webrev: http://cr.openjdk.java.net/~jcbeyler/8212824/webrev.00/ Thanks, Jc

Re: JDK-8211343: nsk_jvmti_parseoptions should handle multiple suboptions

Hi Gary, I had seen that too and forgot to file it! So thanks! I think the comment you removed is interesting, I'm not sure what it means exactly and I've tried re-reading a few times but the use of "in this question" is weird :-) Anyway, the webrev looks good except perhaps for the use of

Re: [PATCH] 8214535: Extend JMap to support parallel and incremental heap scanning

ap –histo. > https://bugs.openjdk.java.net/browse/JDK-8215624 > Is it reasonable ? > Sounded reasonable to me :-) Jc > Thanks! > > > > BRs, > > Lin > > *From:* JC Beyler [mailto:jcbey...@google.com] > *Sent:* Wednesday, December 19, 2018 12:56 AM > *To:* 臧琳 > *C

Re: RFR (XS) 8215495: Always set isCopy

Sounds good to me: For the odd corner case: Webrev: http://cr.openjdk.java.net/~jcbeyler/8215495/webrev.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8215495 Thanks! Jc On Mon, Dec 17, 2018 at 2:39 PM David Holmes wrote: > Hi Jc, > > On 18/12/2018 8:12 am, JC Beyler wrote: >

Re: RFR (XS) 8215495: Always set isCopy

ote: > Hi Jc, > > On 18/12/2018 3:42 am, JC Beyler wrote: > > Hi all, > > > > Could I get a review for this webrev: > > > > Webrev: http://cr.openjdk.java.net/~jcbeyler/8215495/webrev.00/ > > Bug: https://bugs.openjdk.java.net/browse/JDK-82154

Re: RFR (XS) 8215329: Modify ZGC requirement for HeapMonitorThreadTest.java

anity. > >>* @compile HeapMonitorThreadTest.java > >> - * @run main/othervm/native -Xmx512m -agentlib:HeapMonitorTest > >> MyPackage.HeapMonitorThreadTest > >> - * @requires !vm.gc.Z > >> + * @run main/othervm/native -Xmx768m -agentlib:HeapMonitorTest > >> M

RFR (XS) 8215329: Modify ZGC requirement for HeapMonitorThreadTest.java

Hi all, When working on another webrev, I saw this problem: Webrev: http://cr.openjdk.java.net/~jcbeyler/8215329/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8215329 (Basically, from what I understood from an email from Per Liden: - @requires !vm.gc.Z -> ZGC is built in the JDK

Re: RFR (L) 8215160: Normalize spaces for remaining vmTestbase tests

Thanks Alex for the review! Tested & pushed, Jc On Tue, Dec 11, 2018 at 12:21 PM Alex Menkov wrote: > Hi Jc, > > Thanks for the update. > LGTM. > > --alex > > On 12/10/2018 20:39, JC Beyler wrote: > > Hi Alexey, > > > > Thanks for the review! >

Re: RFR (L) 8215161: Remove spaces in assignments for vmTestbase/[a-j]

> Remove spaces > > On Mon, Dec 10, 2018 at 1:04 PM JC Beyler wrote: > >> Hi all, >> >> Could I get a review that adds a space around comparisons for the >> vmTestbase? This is the first of two webrevs to handle this. >> >> Webrev: http://cr.op

RFR (L) 8215161: Remove spaces in assignments for vmTestbase/[a-j]

Hi all, Could I get a review that adds a space around comparisons for the vmTestbase? This is the first of two webrevs to handle this. Webrev: http://cr.openjdk.java.net/~jcbeyler/8215161/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8215161 Thanks, Jc

RFR(L) 8212960: Remove spaces in assignments for remaining vmTestbase

Hi all, Could I get a review that adds a space around comparisons for the vmTestbase? This is the second of two webrevs to handle this. Webrev: http://cr.openjdk.java.net/~jcbeyler/8215160/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8215160 Thanks, Jc

Re: RFR(XS): 8215042: Move serviceability/sa tests from tier1 to tier3.

Hi Leonid, I cannot comment on whether it is a good idea to put the tests in tier3 but the webrev does look good to achieve that :) So LGTM as far as it seems to do what you intended :) Thanks, Jc On Fri, Dec 7, 2018 at 9:53 PM Leonid Mesnik wrote: > Hi > > Could you please review following

Re: RFR 8214572: nsk/jvmti/unit/ForceEarlyReturn/earlyretbase should not suspend the thread when the top frame executes JVMCI code

Hi Daniil, The webrev looks good but I have a few comments and questions (if you don't mind :-)): Comments: - You say that normally the test will be removed from the problem list once the two fixes are done but in this webrev, you've already removed it (I can't see the other case so I can't

RFR (L) 8214502: Add space after/before {} in remaining vmTestbase tests

Hi all, Could I get a review for adding a space around the {} for the vmTestbase. This webrev handles all remaining cases for vmTestbase :-) Here is the webrev and bug: Webrev: http://cr.openjdk.java.net/~jcbeyler/8214502/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8214502 Thanks

Re: JNI - question about jmethodid values.

Thanks, Jc [1] https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/design.html#wp16696 On Mon, Nov 26, 2018 at 11:33 AM Thomas Stüfe wrote: > Hey JC, > > On Mon, Nov 26, 2018 at 8:15 PM JC Beyler wrote: > > > > Hi all, > > > > Just added my two c

RFR (S) 8213721 : [Graal] Tests vmTestbase/nsk/stress/except/except* may be encountering SEGV during out-of-memory conditions

Hi all, Could I get a review for this small change for the except* tests: Webrev: http://cr.openjdk.java.net/~jcbeyler/8213721/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8213721 The bug contains a good explanation of why this is required. After the webrev, the tests no longer

Re: RFR (XS) : problem list HeapMonitorStatIntervalTest.java

Thanks David, done! Jc On Tue, Nov 20, 2018 at 6:31 PM David Holmes wrote: > Hi Jc, > > Looks good and trivial. Please push. > > Thanks, > David > > On 21/11/2018 12:26 pm, JC Beyler wrote: > > Hi all, > > > > Could you please re

Re: RFR (L) 8213501 : Deploy ExceptionJniWrapper for a few tests

Hi all, Anybody motivated to review this? :) Jc On Wed, Nov 7, 2018 at 9:53 PM JC Beyler wrote: > Hi all, > > Could I have a review for the extension and usage of the > ExceptionJniWrapper. This adds lines and filenames to the end of the > wrapper JNI methods, adds tracing, and

Re: [8u] RFR: 8210647: libsaproc is being compiled without optimization

Hi Severin, That does look different ( http://hg.openjdk.java.net/jdk/jdk/rev/c608b2190460) :) It looks good to me :), Jc On Thu, Nov 15, 2018 at 6:58 AM Severin Gehwolf wrote: > Hi, > > Could I please get reviews for this 8u backport of JDK-8210647. Since > the build system is different in

Re: RFR (S) 8212931 HeapMonitorStatIntervalTest.java fails due average calculation

to assess average > sampling interval 113 // via the HeapMonitorStatIntervalTest. > ... > > 125 // Get the actual average size. 126 oneElementSize = > getSize(frames); 127 System.out.println("Element size is: " + > oneElementSize); > > > Thanks, > Se

Re: RFR: JDK-8213916: no copyright in signature.html

Hi Gary, Looks good to me (not a reviewer though), Jc On Thu, Nov 15, 2018 at 5:45 AM Gary Adams wrote: > Here's a quick fix to add a missing copyright to signature.html. > > > diff --git > a/src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html >

Re: What actions are allowed in an JVMTI ResourceExhausted event handler?

It seems what we do with other events that might have this type of "risk" is to defer the event to the ServiceThread, which is a Java thread, no? But perhaps for a resource exhausted just ignoring it for the compiler thread and letting another "Java thread" be aware of it and posting is a better

Re: RFR (XS): 8213525: new unit test GetLocalVariable/LocalVars is not stable

pit...@oracle.com> wrote: > On 11/12/18 20:05, serguei.spit...@oracle.com wrote: > > Hi Jc, > > > Thank you a lot for reviewing! > > On 11/12/18 09:35, JC Beyler wrote: > > Hi Serguei, > > The fix looks good (though I never like commented out code, why do we no

Re: RFR: (S): JDK-8213323: sa/TestJmapCoreMetaspace.java and sa/TestJmapCore.java fail with ZGC

Hi Jini, The webrev looks good to me as well except for a few questions/comments: I have a generic question that is related to the webrev: - Are there plans at some point for Jmap to support ZGC heaps in the future ? Or is this infeasible? I ask because if a lot of tests are disabled

RFR (L) 8213501 : Deploy ExceptionJniWrapper for a few tests

Hi all, Could I have a review for the extension and usage of the ExceptionJniWrapper. This adds lines and filenames to the end of the wrapper JNI methods, adds tracing, and throws an error if need be. I've ported the gc/lock files to use the new TRACE_JNI_CALL add-on and I've ported a few of the

Re: RFR (M) 212939: Add space after if/while/for/switch and parenthesis

ieldName(targetClass, > 407 targetFields[field], > 408 _info[object].fields[field].name, > 409 _info[object].fields[field].signature, > 410 NULL))) > > 481 if ((objects_info[object].fields[field].primitive && > !objects_info[object].collected) > 482 || !obj

Re: RFR (M) 8213003: Remove more assignments in ifs for vmTestbase/[a-s]

since they are pretty > significant. > > > The exception changes need to be discussed after a separate RFR is posted. > > Thanks, > Serguei > > thanks, > > Chris > > On 11/2/18 9:09 PM, JC Beyler wrote: > > Hi Chris, > > Yes that is correct, the webrev i

Re: RFR 8203174: [Graal] JDI tests fail with Unexpected exception: com.sun.jdi.ObjectCollectedException

Hi Daniil, Quick question about looking at the launch options, is there any reason you do not simply use: sun.hotspot.code.Compiler.isGraalEnabled(); Is it because you do not want to specifically call out Graal? Thanks, Jc On Sun, Nov 4, 2018 at 12:41 PM Daniil Titov wrote: > Hi Chris, > >

Re: RFR (XS) 8213246: Fix typo in vmTestbase failuire to failure

Thanks all, pushed :) Jc On Fri, Nov 2, 2018 at 4:25 PM Igor Ignatyev wrote: > LGTM > > -- Igor - a Reviewer > > On Nov 2, 2018, at 4:22 PM, Mikael Vidstedt > wrote: > > > Looks good. > > Cheers, > Mikael - not a Reviewer > > On Nov 2, 2018, at 4:05 P

Re: RFR (M) 8213003: Remove more assignments in ifs for vmTestbase/[a-s]

ss(DEBUGEE_CLASS_NAME); > > to > > ExceptionCheckingJniEnvPtr jni_wrapper(jni); > ... > debugeeClass = jni_wrapper->FindClass(DEBUGEE_CLASS_NAME, > TRACE_JNI_CALL); > > But none of your ExceptionCheckingJni changes are pushed yet, right? > > thanks, > > Chris > >

RFR (XS) 8213246: Fix typo in vmTestbase failuire to failure

Hi all, Could I get a review for a trivial typo fix: Webrev: http://cr.openjdk.java.net/~jcbeyler/8213246/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8213246 Thanks, Jc

RFR (S) 8212931 HeapMonitorStatIntervalTest.java fails due average calculation

Hi all, Could I get a review for a bug in the calculation of the average size of an allocation? The solution is actually not to do the calculation at all. Instead, I just go in the cache and find an object with the right stacktrace and get its size (since all sizes should be equal because they

Re: RFR (M) 8213003: Remove more assignments in ifs for vmTestbase/[a-s]

ew > macros, and a few example uses. You don't need to fix everything. I just > want to get a better feel for the changes and what the implementation would > look like. > > thanks, > > Chris > > On 11/1/18 1:03 PM, JC Beyler wrote: > > Hi Chris, >

RFR (M) 8213003: Remove more assignments in ifs for vmTestbase/[a-s]

Hi all, I worked on updating my script and handling more assignments so I redid a second pass on the same files to get all the cases. Now, on those files the regex "if.* = " no longer shows any cases we would want to fix. Could I get a review for this webrev: Webrev:

RFR (M) 8201655: Add thread-enabled support for the Heap Sampling

Hi all, When working on the heap sampling, I had promised to do the per thread event so here it is! Could I get a review for this: Webrev: http://cr.openjdk.java.net/~jcbeyler/8201655/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8201655 I was thinking of adding GC-dev for the

Re: RFR (M) 8212884: Remove the assignments in ifs for vmTestbase/[a-s]

pit...@oracle.com> wrote: > Hi Jc, > > It looks great! > Your AI-enabled conversion scripts are smart! :) > > Thanks, > Serguei > > On 10/24/18 09:40, JC Beyler wrote: > > Hi all, > > Here is the first webrev to extract assignments from if test clauses. It >

Re: RFR (L) 8212770: Remove spaces before/after () for vmTestbase/jvmti/[s-u]

Hi all, Anybody else want to review this? We can close the book on spaces before/after () once this one goes in :) Jc On Mon, Oct 22, 2018 at 1:37 PM Alex Menkov wrote: > LGTM. > > --alex > > On 10/22/2018 11:29, JC Beyler wrote: > > Hi all, > > > > Here is

Re: RFR (M) 8212884: Remove the assignments in ifs for vmTestbase/[a-s]

you need to add a space after the "if": > > 130 if(!NSK_VERIFY(jvmti != NULL)) { > > In em01t002.cpp, indent of 2nd line isn't correct: > > 74 methodID = jni_env->GetMethodID( > 75 klass, "loadClass", > "(Ljava/lang/St

Re: RFR: JDK-8211013: TESTBUG] nsk/jdb/kill/kill002 wait for message and prompt

Hi Gary, Looks good to me (not an official reviewer though), Jc On Tue, Oct 23, 2018 at 11:08 AM Gary Adams wrote: > Could I get a second reviewer for this change. > > On 10/15/18, 3:16 PM, Chris Plummer wrote: > > Hi Gary, > > > > I think the simple prompt is meant to indicate that execution

Re: RFR JDK-8212151: jdi/ExclusiveBind.java times out due to "bind failed: Address already in use" on Solaris-X64

; Updated fix: > http://cr.openjdk.java.net/~amenkov/exclusiveBind/webrev.01/ > Moved shared code to new Debuggee class. > > --alex > > On 10/19/2018 10:34, JC Beyler wrote: > > Hi Alex, > > > > I remember seeing this same code so went looking for it and saw it in

Re: RFR (M) 8212535: Remove spaces before/after () for vmTestbase/[a-s]*

(char *)storage_ptr, storage_data); > > 162 NSK_COMPLAIN2("objectReferenceCallback: Local storage was > corrupted: %s ,\n\texpected value: %s\n", > 163 (char *)storage_ptr, storage_data); > > thanks, > > Chr

Re: 8196989: Revamp G1 JMX MemoryPool and GarbageCollector MXBean definitions

because > humongous > > // objects are allocated in the "G1 Humongous Space". If > we allowed > > // the G1 default mode "G1 Old Space", notification would > never > > // happen because no objects are alloca

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 wrote: > Thanks David, for the review... > > > -Original Message- > > From: David Holmes > > Sent: Thursday, October 18, 2018 1:20 PM > > To: Fairoz Matte ;

Re: 8196989: Revamp G1 JMX MemoryPool and GarbageCollector MXBean definitions

Hi Paul, I looked at this but it took time for me to "digest" it and I haven't entirely gone through the real GC changes :) My few remarks on the webrev itself are: - http://cr.openjdk.java.net/~phh/8196989/webrev.02/src/hotspot/share/services/memoryService.hpp.udiff.html - There is no

Re: RFR (L) 8212082: Remove the NSK_CPP_STUB macros from remaining vmTestbase/jvmti/[sS]*

Thanks both :) Jc On Fri, Oct 12, 2018 at 5:17 PM Alex Menkov wrote: > Looks good to me. > > --alex > > On 10/12/2018 14:25, JC Beyler wrote: > > Yes they do, they have run on my dev machine and I'm submitting it on > > the submit repo (though that will test build

Re: RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods

> force visibility of any ThreadHeapSampler statics before a Thread > is > > > used, you don’t need OrderAccess. > > > > > > I’d put anything to do with ThreadHeapSampler into its class > definition > > > rather than define them at f

Re: RFR JDK-8195703: BasicJDWPConnectionTest.java: 'App exited unexpectedly with 2'

Hi Alex, - http://cr.openjdk.java.net/~amenkov/BasicJDWPConn/webrev.01/test/lib/jdk/test/lib/apps/LingeredApp.java.udiff.html -> Why not make it javadoc like the other methods of the same file (so @return instead of returns and a second * at the start of the comment)? -

Re: RFR (XL) 8211801: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/scenarios/[A-E]

> nsk_jvmti_setFailStatus(); > >> break; > >> } > >> > >>Could you, please, remove extra space after NULL. > >> > >> > >> In many-many files (especially, in the foleder: > scenarios

RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods

Hi all, When talking with Serguei about JDK-8201655 , we talked about why ThreadHeapSampler has an enabled/disabled when we could have just used the should_post_sampled_object_alloc to begin with. Could I get a review for this: Webrev:

RFR (L) 8211899: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/scenarios/[E-M]

Hi all, I am continuing the NSK_CPP_STUB removal with this next webrev. Webrev: http://cr.openjdk.java.net/~jcbeyler/8211899/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8211899 The change is still straight-forward though, since it is just doing the same NSK_CPP_STUB removal. However

RFR (XS) 8211905: Remove multiple casts for EM06 file

Hi all, Seems like there is an over-zealous casting in the file nsk/jvmti/scenarios/events/EM06/em06t001/em06t001.cpp: jclassName = (jstring) (jstring) (jstring) (jstring) (jstring) (jstring) (jstring) (jstring) (jstring) NSK_CPP_STUB3(CallObjectMethod, jni_env, klass, This webrev fixes that:

Re: RFR JDK-8211292: [TEST] convert com/sun/jdi/DeferredStepTest.sh test

it at the start of the method (only using the local version for the rest of the method); then everything would be in there. But, I'll still say it is a more a question of style :) LGTM, Jc On Fri, Oct 5, 2018 at 12:01 PM Alex Menkov wrote: > Hi Jc, > > > On 10/05/2018 10:34, JC

Re: RFR JDK-8211292: [TEST] convert com/sun/jdi/DeferredStepTest.sh test

Hi Alex, One question and a comment on this: - I thought HashMap was not thread safe so I think you need to synchronize the access to the map threadData - I think your test code could be simplified if you moved it into a helper method (not tested but just for example): +private void next()

Re: RFR (M) 8211261: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/[A-G]*

if the reviewers agree: - Remove the NSK_CPP_STUB* (which is what these refactoring to do) - Remove the NSK_*_VERIFY macros at which point I'll remove that space you saw for NSK_*_VERIFY lines; that will remove the ) in the lines - Move out the assignments - Remove the remaining spaces befo

Re: RFR (M) 8211261: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/[A-G]*

lease update > ForceEarlyReturn/ForceEarlyReturn001/ForceEarlyReturn001.cpp manually - > converted calls contain a lot of unnecessary spaces. > > --alex > > On 09/27/2018 22:15, JC Beyler wrote: > > Hi all, > > > > I continued the NSK_CPP_STUB removal with this webrev: > > &g

Re: RFR (S) 8210842: Handle JNIGlobalRefLocker.cpp

Ping on this, anybody have time to do a review and give a LGTM? Or David if you have time and will to provide an explicit LGTM :) Thanks, Jc On Mon, Sep 24, 2018 at 9:18 AM JC Beyler wrote: > Hi David, > > Sounds good to me, Alex gave me one LGTM, so it seems I'm waiting for an > e

RFR (M) 8211131: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/[G-I]*

Hi all, I continued the NSK_CPP_STUB removal with this webrev: Webrev: http://cr.openjdk.java.net/~jcbeyler/8211131/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8211131 This does the first 50 files of the jvmti subfolder, it is done automatically by the scripts I put in the bug

Re: RFR: JDK-8210984: [TESTBUG] hs203t003 fails with "# ERROR: hs203t003.cpp, 218: NSK_CPP_STUB2 ( ResumeThread, jvmti, thread)"

Hi Gary, Should you not also fail the test if the test fails at popping or resuming the thread? Apart from that looks good to me (I would remark normally that I'm getting rid of the NSK_CPP_STUBs so you could just jump the gun and remove it directly but this is consistent with the current code

Re: RFR (M) 8210689: Remove the multi-line old C style for string literals

Thanks Alex! Could I get a second review/LGTM ? Thanks for your help! Jc On Fri, Sep 21, 2018 at 5:22 PM Alex Menkov wrote: > LGTM. > > --alex > > On 09/21/2018 17:06, JC Beyler wrote: > > Hi Alex, > > > > Good catch, it was not done on purpose bu

Re: RFR (M) 8210689: Remove the multi-line old C style for string literals

d of this output (and there never was). Normally > NSK_COMPLAIN is always used with a terminating \n. > > thanks, > > Chris > > > On 9/21/18 1:05 PM, JC Beyler wrote: > > Hi Chris, > > Sounds good to me; here it is: > Webrev: http://cr.openjdk.java.net/~jcbeyler/821068

Re: RFR (S) 8210842: Handle JNIGlobalRefLocker.cpp

Hi David, Thanks for the quick review and thoughts. I have now a new version here that addresses your comments: Webrev: http://cr.openjdk.java.net/~jcbeyler/8210842/webrev.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8210842 I've also inlined my answers/comments. > > > I've slowly

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

5/09/2018 7:30 AM, Alex Menkov wrote: > > 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, > >>

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

mpiler for > hotspot tests". > > /functions/rawmonitor/rawmonitor.cpp had such wrong statements before, > so they should be revised carefully. > AFAIU if JVMTI dunction is called from some callback where jvmtiEnv is > passed, the passed value should be used. > > --alex

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

Hi Igor, Looks good to me! Jc On Thu, Sep 13, 2018 at 5:08 PM 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 please review the next clean up in jdk testlibrary which removes >

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

Hi Per, I do not know this code but your need to call heapIterationFractionUpdate seems to be a code smell that something else could be fixed, no? Your webrev looks fine to me if I ignore the code smell that comes from having to call the update call: When I look at

Re: RFR (XL) 8210385: Clean up JNI_ENV_ARG and factorize the macros for remaining vmTestbase/jvmti tests

vm)->GetEnv(JNI_ENV_ARG(jvm, (void **) ), > -JVMTI_VERSION_1_1); > +res = jvm->GetEnv((void **) , JVMTI_VERSION_1_1); > if (res != JNI_OK || jvmti == NULL) { > printf("Wrong result of a valid call to GetEnv!\n"); > return JNI_ERR; >

Re: RFR: (S) 8210512: [Testbug] vmTestbase/nsk/jdi/ObjectReference/referringObjects/referringObjects002/referringObjects002.java fails with unexpected size of ClassLoaderReference.referringObjects

Hi David, Looks good to me (I'm not a reviewer but wanted to piggy-back and say I actually learnt quite a bit with the conversation on the original webrev). Thanks! Jc On Mon, Sep 10, 2018 at 2:59 PM serguei.spit...@oracle.com < serguei.spit...@oracle.com> wrote: > Hi David, > > It looks good

Re: RFR (L) 8210429: Clean up JNI_ENV_ARG for vmTestbase/jvmti/Get[G-Z] tests

t; -mid = JNI_ENV_PTR(env)->GetStaticMethodID(JNI_ENV_ARG(env, cl),+ >mid = env->GetStaticMethodID(cl, > name, sig); > > I'd suggest to replace to place the call to just one line. > > > No need for another webrev if you fix it. > > Thanks, > Sergu

RFR (L) 8210429: Clean up JNI_ENV_ARG for vmTestbase/jvmti/Get[G-Z] tests

Hi all, Continuing the removal of the JNI_ENV* macros, I have done the other half of the JVMTI Get[G-Z] methods. The final JVMTI test refactoring will come in a final webrev after this one. The change is straightforward as before, just a bit repetitive: Webrev:

Re: RFR (M) 8210198: Clean up JNI_ENV_ARG for vmTestbase/jvmti/Get[A-F] tests

ame, meth_tab[meth_ind].sig); > 136 } > > In getclsldr003.cpp, getfldecl001.cpp, getfldecl002.cpp, and > getfldecl004.cpp, no need to compare with JNI_TRUE. Just negate the > expression.: > > 98 if (env->IsSameObject(classloader, cl) != JNI_TRUE) { > > Otherwise looks goo

RFR (S) 8208352: Merge HeapMonitorTest and HeapMonitorGCTest code

Hi all, Two of the tests shared code and we could refactor to limit future fixes/changes in both spots. Here is the small webrev that does that: Webrev: http://cr.openjdk.java.net/~jcbeyler/8208352/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8208352 Thank you for the future

Re: RFR JDK-8067354: com/sun/jdi/GetLocalVariables4Test.sh failed

Looks good to me Alex. With the various refactoring that you've done, this is getting really easy to read and understand what is actually being tested :) Jc On Thu, Aug 30, 2018 at 3:13 PM Alex Menkov wrote: > Hi all, > > Please review a fix for >

Re: RFR (S) 8203356: VM Object Allocation Collector can infinite recurse

es from "Red > Hat" and "SAP" to see what I mean. > > I'm not sure about the nbproject changes. I've never seen this file get > updated before. > > thanks, > > Chris > > On 8/22/18 4:20 PM, JC Beyler wrote: > > Hi all, > > Would anyone

Re: 8209585: [Graal] vmTestbase/nsk/jvmti/scenarios/sampling tests fail with "Too small stack of resumed thread"

> Webrev: http://cr.openjdk.java.net/~dtitov/8209585/webrev.02/ > Issue: https://bugs.openjdk.java.net/browse/JDK-8209585 > > Thanks! > Daniil > > > > From: serviceability-dev on > behalf of Daniil Titov > Date: Monday, August 27, 2018 at 11:05 AM > To: JC Beyler , &g

Re: RFR: 8209585: [Graal] vmTestbase/nsk/jvmti/scenarios/sampling tests fail with "Too small stack of resumed thread"

oncern. > But now I think there is no point to take these values on non-suspended > threads. > It has to be good enough to compare the values taken on suspended threads > only. > > Thanks, > Serguei > > > On 8/24/18 16:49, JC Beyler wrote: > > Hi

Re: RFR: 8209585: [Graal] vmTestbase/nsk/jvmti/scenarios/sampling tests fail with "Too small stack of resumed thread"

Hi Daniil, Just my two cents about this :) I was looking at this and wondered if it made sense to fix the test this way (I always worry about simplifying a test and losing coverage). I understand the bug is that it is possible that between both calls, Graal could add some frames and therefore

Re: RFR (S) 8203356: VM Object Allocation Collector can infinite recurse

ring the GetClassSignature call")); > 63 } > > > The JVMTI error code, returned by GetClassSignature has to be checked, not > JNI ExceptionOccurred. > Also, I'd suggest to check for signature to be non-NULL. > > Also, the indent in the VMEventRecursionTest.java has

Re: custom/fast heap dumper

For what it's worth, when possible, I think it's easier to use the heap samples to understand leaks (it's often what our users end up doing; ie: hmm why is this heap allocation still remaining live?). You could also dump the heap samples at OOM and/or VM exit. I know it is not always comparable

Re: RFR (S) 8201224: Make string buffer size dynamic

r the limit as I misread the return of the method. You are right, sorry about that. Thanks again! Jc > > --alex > > On 08/10/2018 07:55, JC Beyler wrote: > > Hi all, > > > > Anybody motivated to look at this change? :) > > > > Webrev: http://cr.openj

Re: RFR (S) 8201224: Make string buffer size dynamic

Hi all, Anybody motivated to look at this change? :) Webrev: http://cr.openjdk.java.net/~jcbeyler/8201224/webrev.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8201224 Let me know what you think! Jc On Wed, Aug 8, 2018 at 11:34 AM JC Beyler wrote: > Hi all, > > Here is a revis

Re: RFR 8208303: Track JNI failures and fail tests

tead > "fill_native_frames" > No need to resent webrev. > > --alex > > On 08/08/2018 10:56, JC Beyler wrote: > > Hi Serguei, > > > > Sounds good to me. If someone else could review it, I could add the > > metadata to the webrev and, once testin

Re: RFR(S) 8207765: HeapMonitorIntervalRateTest fails with ZGC

idn't see this thread until now... been on vacation, jvmls, etc) > > On 07/19/2018 11:52 PM, JC Beyler wrote: > > Hi Serguei, > > > > Done here: > > http://cr.openjdk.java.net/~jcbeyler/8207765/webrev.01/ > > > > I added: > > > > + // Cal

RFR (S) 8203356: VM Object Allocation Collector can infinite recurse

Hi all, (Renaming the thread that did not have the RFR in front of the subject, I apologize) Could someone review this change: Webrev: http://cr.openjdk.java.net/~jcbeyler/8203356/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8203356 Basically, if during a callback from a

Re: RFR 8208303: Track JNI failures and fail tests

e)? > > Thanks, > Serguei > > > On 7/26/18 12:08, Daniel D. Daugherty wrote: > > Please make sure this fix is well tested in Mach5 prior to pushing. > In particular, I'm focused on reducing the noise in Mach5 tier[1-3] > so adding any new failures there will ma

RFR (XS) 8208251: serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorGCCMSTest.java fails intermittently on Linux-X64

Hi all, As we fixed the HeapMonitorTest to not fail from time to time, there seems to be the same issue and risk in HeapMonitorGCTest. Could someone review the similar fix: Webrev: http://cr.openjdk.java.net/~jcbeyler/8208251/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8208251 The

Re: RFR (XS) 8208059: [TESTBUG] HeapMonitorInterpreterArrayTest.java fails

I'll push it after you send me a patch. > > > On 7/25/18 07:42, Daniel D. Daugherty wrote: > > On 7/25/18 10:20 AM, JC Beyler wrote: > > Hi all, > > There seems to be an intermittent failure with the > HeapMonitorInterpreterArrayTest. I believe it is due to the possibil

Re: RFR (XS) 8208059: [TESTBUG] HeapMonitorInterpreterArrayTest.java fails

Thanks for your help Daniel, Could I get a second review and I'll prepare an updated webrev :) Jc On Wed, Jul 25, 2018 at 7:42 AM Daniel D. Daugherty < daniel.daughe...@oracle.com> wrote: > On 7/25/18 10:20 AM, JC Beyler wrote: > > Hi all, > > There seems to be an

Re: RFR (S) 8207252: C1 still does eden allocations when TLAB is enabled

> Thanks, > >> Serguei > >> > >> > >> On 7/19/18 00:48, Rahul Raghavan wrote: > >>> RFR (S) 8207252: C1 still does eden allocations when TLAB is enabled > >>> > >>> (just adding + hotspot-compiler-dev also) > >>> >

RFR(S) 8207765: HeapMonitorIntervalRateTest fails with ZGC

Hi all, Could I have a few reviews of: http://cr.openjdk.java.net/~jcbeyler/8207765/webrev.00/ The test assumed the size of a 1-element array but ZGC changes that assumption. The test now first allocates a bit of memory and gets the average size of the samples before assuming the size. This

Re: RFR(S) 8205652: serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatRateTest.java fails

gt; > > It looks good to me. > > > > Thanks, > > Serguei > > > > On 7/16/18 10:58, JC Beyler wrote: > >> Hi all, > >> > >> Small RFR to update a HeapMonitor test that had two issues: a test was > >> wrong and the test was not all

Re: RFR(S) 8205541: serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatArrayCorrectnessTest.java fails

t...@oracle.com wrote: > > Hi Jc, > > > > It looks good to me. > > > > Thanks, > > Serguei > > > > > > On 7/16/18 12:37, JC Beyler wrote: > >> Hi all, > >> > >> Small RFR to update two HeapMonitor tests to remove test fail

RFR(S) 8205652: serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatRateTest.java fails

Hi all, Small RFR to update a HeapMonitor test that had two issues: a test was wrong and the test was not allocating enough to get to an expected sample count. Instead of allocating 10 times more and hit some OOM on the test framework, the webrev allocates in chunks and gets the number of

Re: RFR (S) 8206960: [Graal] serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor tests fail

> > Thanks, > Serguei > > > On 7/12/18 11:30, Alex Menkov wrote: > > Looks good to me as well. > > > > --alex > > > > On 07/12/2018 07:25, JC Beyler wrote: > >> Thanks Serguei! > >> > >> Anybody motivated to give this a

Re: RFR (S) 8206960: [Graal] serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor tests fail

Serguei > > > On 7/11/18 10:04, JC Beyler wrote: > > Hi all, > > Could someone review the small-ish webrev for the bug: > https://bugs.openjdk.java.net/browse/JDK-8206960 > > The webrev is here: > http://cr.openjdk.java.net/~jcbeyler/8206960/webrev.00/ > > Ba

Re: RFR (S) 8205643: [Graal] serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorGCCMSTest.java fails

mList.txt > > > > Thanks, > > Serguei > > > > > > On 7/10/18 12:26, Alex Menkov wrote: > >> Hi JC, > >> > >> you need also to remove the test from ProblemList > >> > >> --alex > >> > >> On 0

Re: RFR 8171119: Low-Overhead Heap Profiling

Robbin Ehn <robbin@oracle.com> wrote: > On 2018-05-14 21:37, JC Beyler wrote: > > Hi Robbin, > > > > I did this then, which should be explicit enough, no? > > > > // If we want to be sampling, protect the allocated object with > a Hand

Re: RFR 8171119: Low-Overhead Heap Profiling

(or a inline.hpp). > > As general note, not your doing, setting a pointer in a heap allocated > object to > a stack allocated object is a really bad pattern. > JvmtiThreadState -> collector > > Thanks, Robbin > > On 05/08/2018 03:10 AM, JC Beyler wrote:

  1   2   >