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