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 https://bugs.ope

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

line 212. It is not clear from scratch how these two values 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 ex

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 tha

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 add

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 i

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 PM,

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

t; > > Since the same check is already done in the "inner" load_shared_class() > method. I ran into issue with the find_shared_class() before the inner load_shared_class() is called earlier. That’s why the check was done in both load_shared_class. With updated approach, we m

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

uld just skip 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 >

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

L-> 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 locally, so it also could be inside the #if INCLUDE_CDS. >

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

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, Jiangli Zhou wrote: &

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 par

Re: RFR 8150607 - Clean up CompactHashtable

Ioi, > On Apr 11, 2016, at 7:11 PM, Ioi Lam 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 table with >> Simp

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 the

Re: RFR 8081219: hs_err improvement: Add event logging for class redefinition to the hs_err file

Hi Coleen, The change looks good to me. Thanks, Jiangli On Jun 2, 2015, at 12:58 PM, Coleen Phillimore wrote: > Summary: Use the Events::log function to save redefined classes for output to > the hs_err file. > > This change adds lines to the hs_err_pid file which names classes redefined: >

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 in

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

tring is not in the symbol > + 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

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 wrote: > Hi Ioi, > > Thanks for the review. > > On Jan 30, 2015, at 9:49 AM, Ioi Lam wrote: &

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

l lookup issue. 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.

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

but it's 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, &g

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

return sym; > } >} > } > -return null; > + > +sym = sharedTable.probe(name, hashValue); > +return sym; >} > > > You may also want to update the copyright comments. Yep, will do. David also sent me an offline reminde

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 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.

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 Z

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 table

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 <ht

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" To: "Jiangli Zhou" , "hotspot-run

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

Hi, Please review the fix for 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 incl

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. Reviewed-by

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 wrote: Hi, Please review the fix for 8023477: http://cr.openjdk.java.net/~jiangli/8023477/webrev.00/ Both tests reported by the bug failed due to

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

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

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 mapp

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

ees the problem, let me know. /Staffan On 12 aug 2013, at 22:45, Jiangli Zhou <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 touc

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

el_class_loading.testlist as well. Thanks - Ioi On 08/09/2013 02:22 PM, Jiangli Zhou wrote: Hi, Could anyone help me review this? Thanks, Jiangli On 07/31/2013 01:51 PM, Jiangli Zhou wrote: Hi, Please review following change for JDK-8021948 <https://jbs.oracle.com/bugs/browse/JDK-8021948&g

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 InstanceKlass::

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 ! src/sh

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

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.jd

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

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

u 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

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

wrote: Hi 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:

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 & A

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.

hat 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 wrote: Hi David, It was an urgent fix and didn't go through the "normal" route. Thanks, Jiangli On

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. Summ

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

Changeset: 44c5fcd9cb25 Author:iklam Date: 2013-01-24 10:57 -0800 URL: http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/44c5fcd9cb25 8006280: Need to reorder metadata structures to reduce size (64-bit) Summary: Reordered Klass, InstanceKlass and Method to save 8 bytes each Revi

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 implem