RFR 8071962 The SA code needs to be updated to support Symbol lookup from the shared archive
Jiangli Zhou
jiangli.zhou at oracle.com
Fri Jan 30 23:12:06 UTC 2015
Thank you, Yumin. I incorporated your following suggestions with slight modification for the comment change.
Thanks again,
Jiangli
On Jan 30, 2015, at 2:55 PM, Yumin Qi <yumin.qi at oracle.com> wrote:
> Jiangli,
>
> Two minor suggestions (did not pay attention in first round, anyway, no harm):
>
> 1) CompactHashtable.java:
>
> 58 private static long uintxSize;
>
> I did not find the usage of this var, please remove it.
>
> 2)
> + anyway. Searches the regular symbol table and the shared symbol
> + table. Null is returned if the given string 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 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 at oracle.com> wrote:
>>
>>> Hi Ioi,
>>>
>>> Thanks for the review.
>>>
>>> On Jan 30, 2015, at 9:49 AM, Ioi Lam <ioi.lam at 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 at 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
>>>>>>>
>>>>>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20150130/91596af7/attachment.html>
More information about the serviceability-dev
mailing list