Jiangli, Looks good for me!
-Dmitry On 2015-01-31 01:31, Jiangli Zhou wrote: > 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....@oracle.com> wrote: >> >>> Hi Jiangli, >>> >>> The code looks good. I am wondering if you need a more specific JTREG test >>> for the new sun/jvm/hotspot/utilities/CompactHashTable.java. There's a lot >>> of code in CompactHashTable.java. Will the existing test case in 8071962 >>> provide enough coverage? >> >> My thinking would be yes. The symbol lookup is very foundational operations >> in sun.jvm.hotspot.tools.DumpJFR. If it fails, it would get exception >> immediately. We also have JTREG tests that uncovers the SA symbol 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.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: >>>>>> 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. The VM looks up both tables when searching for >>>>>> existing symbol at runtime. The SA code needs to do the same when >>>>>> looking up symbols. >>>>>> >>>>>> Webrev: http://cr.openjdk.java.net/~jiangli/8067977/webrev.00/ >>>>> Jiangli, >>>>> >>>>> I'm not reviewing, just wanted to make sure there is no typo here ... >>>>> The webrev bug number is 8067977, but the RFR bug number is 8071962 which >>>>> is strange. >>>>> >>>>> Thanks, >>>>> Serguei >>>>> >>>>>> Tested on both 32-bit and 64-bit platforms: >>>>>> bin/java sun.jvm.hotspot.tools.DumpJFR <pid> >>>>>> >>>>>> JPRT tests in progress. >>>>>> >>>>>> Thanks, >>>>>> Jiangli >>>>>> >>>>>> >>> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the source code.
