[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR 8071962 The SA code needs to be updated to support Symbol lookup from the shared archive
From:       Dmitry Samersoff <dmitry.samersoff () oracle ! com>
Date:       2015-01-31 7:22:46
Message-ID: 54CC82C6.6070000 () oracle ! com
[Download RAW message or body]

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.zhou@oracle.com> wrote:
> 
> > Hi Ioi,
> > 
> > Thanks for the review.
> > 
> > On Jan 30, 2015, at 9:49 AM, Ioi Lam <ioi.lam@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.spitsyn@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.


[prev in list] [next in list] [prev in thread] [next in thread]