Re: RFR (T) 8215034: Remove old HOTSWAP conditionals

Looks good and trivial. Thanks, Jiangli On 12/7/18 1:57 PM, coleen.phillim...@oracle.com wrote: I was in the area and found these few remaining conditionals. Tested with tier1 on Oracle platforms. open webrev at http://cr.openjdk.java.net/~coleenp/8215034.01/webrev bug link

Re: RFR(M): 8201375: Add the AllowArchivingWithJavaAgent diagnostic vm option to allow the use of the -javaagent option during CDS dumping

can be different at line 1350. At least, some comment explaining it is needed. Thanks, Serguei On 11/15/18 10:56, Jiangli Zhou wrote: +1 Adding serviceability-dev mailing list. It would be good to have this reviewed by the JVMTI experts also to make sure all cases are covered. Thanks, J

Re: RFR(M): 8201375: Add the AllowArchivingWithJavaAgent diagnostic vm option to allow the use of the -javaagent option during CDS dumping

+1 Adding serviceability-dev mailing list. It would be good to have this reviewed by the JVMTI experts also to make sure all cases are covered. Thanks, Jiangli On 11/15/18 9:29 AM, Ioi Lam wrote: Hi Calvin, The changes look good. Thanks - Ioi On 11/14/18 4:40 PM, Calvin Cheung wrote:

Re: RFR(S) 8212200 assert when shared java.lang.Object is redefined by JVMTI agent

Sounds good. Thanks for trying that. Thanks, Jiangli > On Oct 22, 2018, at 6:43 PM, Ioi Lam wrote: > > > >> On 10/22/18 3:06 PM, Jiangli Zhou wrote: >>> On 10/22/18 10:56 AM, Ioi Lam wrote: >>> >>> >>> >>>> On 10/22/1

Re: RFR(S) 8212200 assert when shared java.lang.Object is redefined by JVMTI agent

On 10/22/18 10:56 AM, Ioi Lam wrote: On 10/22/18 10:25 AM, Jiangli Zhou wrote: Hi Ioi, Looks good. Please see comments below. - src/hotspot/share/classfile/javaClasses.cpp 4254 assert(JvmtiEnvBase::get_phase() <= JVMTI_PHASE_PRIMORDIAL, 4255    "Field offsets of we

Re: RFR(S) 8212200 assert when shared java.lang.Object is redefined by JVMTI agent

On 10/21/18 6:15 PM, Ioi Lam wrote: Re-sending to the correct mailing lists. Please disregard the other email. http://cr.openjdk.java.net/~iklam/jdk12/8212200-cds-jvmti-clfh-critical-classes.v03/ https://bugs.openjdk.java.net/browse/JDK-8212200 Hi, CDS has various built-in assumptions

Re: RFR(S) 8212200 assert when shared java.lang.Object is redefined by JVMTI agent

Hi Ioi, Looks good. Please see comments below. - src/hotspot/share/classfile/javaClasses.cpp 4254 assert(JvmtiEnvBase::get_phase() <= JVMTI_PHASE_PRIMORDIAL, 4255    "Field offsets of well-known classes must be computed in JVMTI_PHASE_PRIMORDIAL or before"); Maybe it is worth

Re: RFR: 8203031: segfaults from jvmti_AddToBootstrapClassLoaderSearch

+1 Thanks for fixing it! Jiangli > On May 30, 2018, at 4:29 PM, Ioi Lam wrote: > > Hi Alex, > > The changes look good. Thank you so much for fixing it. > > - Ioi > > >> On 5/30/18 3:57 PM, serguei.spit...@oracle.com wrote: >> Hi Alex, >> >> The fix looks good to me. >> >> It'd be nice

Re: RFR [XS] Subclasses of jdk.jfr.Event loaded from CDS breaks -XX:FlightRecorderOptions=retransform=false

Hi Ioi, SystemDictionary::reorder_dictionary_for_sharing() and Dictionary::reorder_dictionary_for_sharing() are only used for CDS code. Could you please add CDS_ONLY() to the function definitions and put the implementation under #if INCLUDE_CDS. Thanks, Jiangli > On Oct 26, 2017, at 4:26

Re: RFR: 8141341: CDS should be disabled if JvmtiExport::should_post_class_file_load_hook() is true

ith updated approach, we might no longer need the check here. Thanks, Jiangli > > Thanks > - Ioi > > > On 7/8/16 11:18 AM, Jiangli Zhou wrote: >> Hi Karen, >> >> Thank you for the feedback. I think Ioi and your comments regarding the >> dynamically a

Re: RFR: 8141341: CDS should be disabled if JvmtiExport::should_post_class_file_load_hook() is true

kip the CFLH events for the shared classes > if the capability can_generate_all_class_hook_events is not enabled and > continue posting the events for non-shared classes. Then we do not need to > tweak the SetEventNotificationMode() implementation. Otherwise, the fragment > above

Re: RFR: 8141341: CDS should be disabled if JvmtiExport::should_post_class_file_load_hook() is true

inor code review comments: > > metaspace.cpp line 3181: CFHL-> CFLH > > metaspace.cpp: > I appreciated your moving the if (UseSharedSpaces) into the #INCLUDE_CDS. > I think it would make sense to do a bit more of that - e.g. cds_address > appears to only be > use loca

Re: RFR: 8141341: CDS should be disabled if JvmtiExport::should_post_class_file_load_hook() is true

l agent that requires > JVMTI_EVENT_CLASS_FILE_LOAD_HOOK cannot be dynamically loaded after JVM has > started"); > + return JVMTI_ERROR_ILLEGAL_ARGUMENT; > + } >record_class_file_load_hook_enabled(); > } > ... > } > > > Thanks > - Ioi > > > > On 7/7/16 6:08 PM, Jiang

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

ei > > > On 6/22/16 10:29, Jiangli Zhou wrote: >> Hi Serguei, >> >> The comment in the following assert is outdated. There is no >> get_module_from_pkg. Should it be changed to get_module_by_package_name()? >> >> 804 assert(ModuleEntryTable::j

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

Hi Serguei, The comment in the following assert is outdated. There is no get_module_from_pkg. Should it be changed to get_module_by_package_name()? 804 assert(ModuleEntryTable::javabase_defined(), 805 "Attempt to call get_module_from_pkg before java.base is defined"); It’s not

Re: RFR 8150607 - Clean up CompactHashtable

Ioi, > On Apr 11, 2016, at 7:11 PM, Ioi Lam <ioi@oracle.com> wrote: > > Hi Jiangli, > > > Thanks for the review: > > On 4/11/16 6:44 PM, Jiangli Zhou wrote: >> Hi Ioi, >> >> I like the more structural way of reading/writing the compact

Re: RFR 8150607 - Clean up CompactHashtable

Hi Ioi, I like the more structural way of reading/writing the compact table with SimpleCompactHashtable. It looks quite clean overall. - How about using VALUE_ONLY_BUCKET_TYPE, which is more descriptive than TINY_BUCKET_TYPE? - The following assert in CompactSymbolTableWriter::add() limits

Re: RFR 8071962 The SA code needs to be updated to support Symbol lookup from the shared archive

+ tables. */ Maybe better with: Return null if the given name is not found in both tables. ? No need to send review again if you change. Thanks Yumin On 1/30/2015 2:31 PM, Jiangli Zhou wrote: Here is the updated webrev that incorporates all feedbacks: http://cr.openjdk.java.net

Re: RFR 8071962 The SA code needs to be updated to support Symbol lookup from the shared archive

Here is the updated webrev that incorporates all feedbacks: http://cr.openjdk.java.net/~jiangli/8071962/webrev.01/ Thanks, Jiangli On Jan 30, 2015, at 10:05 AM, Jiangli Zhou jiangli.z...@oracle.com wrote: Hi Ioi, Thanks for the review. On Jan 30, 2015, at 9:49 AM, Ioi Lam ioi

Re: RFR 8071962 The SA code needs to be updated to support Symbol lookup from the shared archive

, hashValue); +return sym; } You may also want to update the copyright comments. Yep, will do. David also sent me an offline reminder for the copyright headers. Thanks! Jiangli Thanks, Serguei On 1/29/15 5:46 PM, Jiangli Zhou wrote: Hi Serguei, Thanks for noticing that. It’s

Re: RFR 8071962 The SA code needs to be updated to support Symbol lookup from the shared archive

not a concern - you can leave it as is. Thanks you for the suggestion. Sounds good to me. I’ll make the change and post a new webrev that incorporates everyone’s feedback. Thanks! Jiangli -Dmitry On 2015-01-30 04:13, Jiangli Zhou wrote: Hi, Please review the change for JDK-8071962, The SA code

Re: RFR 8071962 The SA code needs to be updated to support Symbol lookup from the shared archive

. Thanks, Jiangli Thanks - Ioi On 1/29/15, 5:46 PM, Jiangli Zhou wrote: Hi Serguei, Thanks for noticing that. It’s actually a duplicated bug ID for the same issue. I just fixed the web rev: http://cr.openjdk.java.net/~jiangli/8071962/webrev.00/. Thanks, Jiangli On Jan 29, 2015

Re: RFR 8071962 The SA code needs to be updated to support Symbol lookup from the shared archive

Hi Serguei, Thanks for double checking it. Pushing the change … Thanks, Jiangli On Jan 30, 2015, at 5:12 PM, serguei.spit...@oracle.com wrote: Looks good. Thanks, Serguei On 1/30/15 2:31 PM, Jiangli Zhou wrote: Here is the updated webrev that incorporates all feedbacks: http

Re: RFR 8071962 The SA code needs to be updated to support Symbol lookup from the shared archive

Hi Serguei, Thanks for noticing that. It’s actually a duplicated bug ID for the same issue. I just fixed the web rev: http://cr.openjdk.java.net/~jiangli/8071962/webrev.00/. Thanks, Jiangli On Jan 29, 2015, at 5:40 PM, serguei.spit...@oracle.com wrote: On 1/29/15 5:13 PM, Jiangli Zhou wrote

Re: RFR 8071962 The SA code needs to be updated to support Symbol lookup from the shared archive

Hi Yumin, Thank you for the quick review! Jiangli On Jan 29, 2015, at 6:03 PM, Yumin Qi yumin...@oracle.com wrote: Looks good to me. Not a 'R'eviwer. Thanks Yumin On 1/29/2015 5:46 PM, Jiangli Zhou wrote: Hi Serguei, Thanks for noticing that. It’s actually a duplicated bug ID

RFR 8071962 The SA code needs to be updated to support Symbol lookup from the shared archive

Hi, Please review the change for JDK-8071962, The SA code needs to be updated to support Symbol lookup from the shared archive”. In JDK-8059510, we introduced a compact form of hash table for the symbols in shared archive. The shared symbols are stored separately from the regular symbol

Re: RFR 8067982: some jcmd /gc/heap_dump tests failed: hprof output contains warning or error

Thanks, Fredrik! Jiangli On 01/14/2015 12:06 AM, fredrik.arvids...@oracle.com wrote: Hi Looks good to me (not a reviewer). Thanks for fixing this! /Fredrik Sent from my HTC - Reply message - From: Yumin Qi yumin...@oracle.com To: Jiangli Zhou jiangli.z...@oracle.com, hotspot-runtime

Re: RFR 8067982: some jcmd /gc/heap_dump tests failed: hprof output contains warning or error

Hi Yumin, Thanks for the review! Will fix the copyright. Thanks, Jiangli On 01/13/2015 07:56 PM, Yumin Qi wrote: Looks good to me. Could you update the copyright year? Thanks Yumin On 1/13/2015 4:50 PM, Jiangli Zhou wrote: Hi, Please review the fix for JDK-8067982 https

RFR 8067982: some jcmd /gc/heap_dump tests failed: hprof output contains warning or error

Hi, Please review the fix for JDK-8067982 https://bugs.openjdk.java.net/browse/JDK-8067982 (Some jcmd /gc/heap_dump tests failed: hprof output contains warning or error). The jcmd heap_dump tests failed due to incomplete symbol information in the dump output. The shared symbols are not

hg: hsx/hotspot-rt/hotspot: 2 new changesets

Changeset: 6b3ac96bada6 Author:jiangli Date: 2013-08-26 13:32 -0400 URL: http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/6b3ac96bada6 8023477: Invalid CP index when reading ConstantPool. Summary: Need to check for 0 case for InstanceKlass::_generic_signature_index.

Re: Request for review:8023477: Invalid CP index when reading ConstantPool

Thanks, Staffan! Jiangli On 08/26/2013 01:13 AM, Staffan Larsen wrote: Looks good. Thanks, /Staffan On 23 aug 2013, at 23:49, Jiangli Zhou jiangli.z...@oracle.com wrote: Hi, Please review the fix for 8023477: http://cr.openjdk.java.net/~jiangli/8023477/webrev.00/ Both tests reported

Request for review:8023477: Invalid CP index when reading ConstantPool

. Thanks, Jiangli On 08/23/2013 11:00 AM, Jiangli Zhou wrote: I found the issue in the SA code. For class without generic signature, the InstanceKlass::_generic_signature_index is 0. Need to check for this case in the SA code. I'm working on the fix and doing testing. Thanks, Jiangli On 08/23

Re: Request for review:8023477: Invalid CP index when reading ConstantPool

Thanks again, Serguei! Jiangli On 08/23/2013 03:27 PM, serguei.spit...@oracle.com wrote: Hi Jiangli, The fix looks good and safe. Thanks, Serguei On 8/23/13 2:49 PM, Jiangli Zhou wrote: Hi, Please review the fix for 8023477: http://cr.openjdk.java.net/~jiangli/8023477/webrev.00/ Both

Re: Request for review:8021948:Change InstanceKlass::_source_file_name and _generic_signature from Symbol* to constant pool index

2013, at 22:45, Jiangli Zhou jiangli.z...@oracle.com mailto:jiangli.z...@oracle.com wrote: Hi Ioi, Thanks for the review! I'll try those tests also. Thanks, Jiangli On 08/12/2013 11:09 AM, Ioi Lam wrote: Looks good. Since you're touching the class loader code, maybe you should run

Re: Request for review:8021948:Change InstanceKlass::_source_file_name and _generic_signature from Symbol* to constant pool index

the problem, let me know. /Staffan On 12 aug 2013, at 22:45, Jiangli Zhou jiangli.z...@oracle.com mailto:jiangli.z...@oracle.com wrote: Hi Ioi, Thanks for the review! I'll try those tests also. Thanks, Jiangli On 08/12/2013 11:09 AM, Ioi Lam wrote: Looks good. Since you're touching the class

hg: hsx/hotspot-rt/hotspot: 2 new changesets

Changeset: ff2520b97b00 Author:jiangli Date: 2013-08-22 19:27 -0400 URL: http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/ff2520b97b00 8023547: com/sun/jdi/RedefineMulti.sh fails with IllegalArgumentException after JDK-8021948 . Summary: Need to check if the constant pool

hg: hsx/hotspot-rt/hotspot: 2 new changesets

Changeset: e22ee8e7ae62 Author:jiangli Date: 2013-08-19 14:59 -0400 URL: http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/e22ee8e7ae62 8021948: Change InstanceKlass::_source_file_name and _generic_signature from Symbol* to constant pool indexes. Summary: Change

hg: hsx/hotspot-rt/hotspot: 2 new changesets

Changeset: 825e6cb66923 Author:jiangli Date: 2013-07-17 18:06 -0400 URL: http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/825e6cb66923 8020309: Eliminate InstanceKlass::_cached_class_file_len. Summary: Use JvmtiCachedClassFileData. Reviewed-by: iklam, sspitsyn, dcubed !

Re: Request for review:8020309:Remove InstanceKlass::_cached_class_file_len and record the length together with the cached class file bytes

for the update! It makes sense to also run the JTREG java/lang/instrument tests. Thanks, Serguei On 7/12/13 1:55 PM, Jiangli Zhou wrote: Hi Ioi and Serguei, Here is the updated the webrev: http://cr.openjdk.java.net/~jiangli/8020309/webrev.01/. I've retested with nsk.jvmti, nsk.jdi and nsk.hprof

Re: Request for review:8020309:Remove InstanceKlass::_cached_class_file_len and record the length together with the cached class file bytes

On 07/15/2013 01:58 PM, Jiangli Zhou wrote: Hi Serguei, Thanks for the review! I've rerun all following tests and JPTR: jdk/test/java/lang/instrument jdk/test/com/sun/jdi nsk.jvmti nsk.jdi nsk.hprof And vm.quick.testlist as well. Thanks, Jiangli Thanks, Jiangli On 07

Re: Request for review:8020309:Remove InstanceKlass::_cached_class_file_len and record the length together with the cached class file bytes

Thanks, Ioi! Jiangli On 07/15/2013 03:05 PM, Ioi Lam wrote: Looks good to me. Thanks - Ioi On 07/12/2013 01:55 PM, Jiangli Zhou wrote: Hi Ioi and Serguei, Here is the updated the webrev: http://cr.openjdk.java.net/~jiangli/8020309/webrev.01/. I've retested with nsk.jvmti, nsk.jdi

Re: Request for review:8020309:Remove InstanceKlass::_cached_class_file_len and record the length together with the cached class file bytes

Hi Dan, Thanks for the review! On 07/15/2013 03:45 PM, Daniel D. Daugherty wrote: On 7/12/13 2:55 PM, Jiangli Zhou wrote: Hi Ioi and Serguei, Here is the updated the webrev: http://cr.openjdk.java.net/~jiangli/8020309/webrev.01/. Thumbs up! src/share/vm/classfile/classFileParser.cpp

Re: Request for review:8020309:Remove InstanceKlass::_cached_class_file_len and record the length together with the cached class file bytes

Jiangli, I like the Ioi's sugestion. It will help to avoid the manipulations with offsets that look as unnecessary complexity. Thanks, Serguei On 7/10/13 6:39 PM, Jiangli Zhou wrote: Hi Ioi, That's good suggestion. Let me look into it. Thanks! Jiangli On 07/10/2013 05:53 PM, Ioi Lam wrote

hg: hsx/hotspot-rt/hotspot: 3 new changesets

Changeset: 71180a6e5080 Author:jiangli Date: 2013-07-03 17:26 -0400 URL: http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/71180a6e5080 7133260: AllocationProfiler uses space in metadata and doesn't seem to do anything useful. Summary: Remove -Xaprof and Klass::_alloc_count

Re: hg: hsx/hotspot-rt/hotspot: 8012927: 'assert(nbits == 32 || (-(1 nbits-1) = x x ( 1 nbits-1))) failed: value out of range' in interpreter initialization.

that an open review system (which I also hope is coming soon) makes it less likely that this happens again. -- Chris Regards, Volker On Wed, Apr 24, 2013 at 6:26 PM, Jiangli Zhou jiangli.z...@oracle.com wrote: Hi David, It was an urgent fix and didn't go through the normal route. Thanks, Jiangli

Re: hg: hsx/hotspot-rt/hotspot: 8012927: 'assert(nbits == 32 || (-(1 nbits-1) = x x ( 1 nbits-1))) failed: value out of range' in interpreter initialization.

Hi David, It was an urgent fix and didn't go through the normal route. Thanks, Jiangli On 04/23/2013 09:32 PM, David Holmes wrote: Jiangli, I do not see a public Request for Review for this change. David On 24/04/2013 9:11 AM, jiangli.z...@oracle.com wrote: Changeset: 1ea6a35dcbe5 Author:

hg: hsx/hotspot-rt/hotspot: 8012927: 'assert(nbits == 32 || (-(1 nbits-1) = x x ( 1 nbits-1))) failed: value out of range' in interpreter initialization.

Changeset: 1ea6a35dcbe5 Author:jiangli Date: 2013-04-23 12:32 -0400 URL: http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/1ea6a35dcbe5 8012927: 'assert(nbits == 32 || (-(1 nbits-1) = x x ( 1 nbits-1))) failed: value out of range' in interpreter initialization. Summary:

Request for review: 7154670: The instanceKlass _implementors[] and _nof_implementors are not needed for non-interface klass

Hi, Please review following webrev that changes the instanceKlass::_implementors[] to an embedded instanceKlass field: http://cr.openjdk.java.net/~jiangli/7154670/webrev.00/ The embedded implementor field is only created for interfaces. Since the VM/compiler does not seem to use the