Looks good.
Thanks,
Serguei
On 1/30/15 2:31 PM, 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
