Need Second Reviewer Re: RR(S): JDK-7127191 SA JSDB does not display native symbols correctly for transported Linux cores

[serguei.spitsyn at oracle.com]

Hi Dmitry,


agent/src/os/linux/libproc_impl.c

Q: To the lines 56-71 (I'm taking a blame for asking stupid questions):
      Is it possible the name argument has no leading slash '/' ?
      Does it work correctly in such a case?
      Is there a need to add a slash after the alt_path?


agent/src/share/classes/sun/jvm/hotspot/debugger/linux/LinuxCDebugger.java

Something is wrong with the comment as it is not clear:

   59     /* Typically we don't have too much loaded object here,
   60        so final efforts to do a linear search less then sort and do
   61        binary serach */

Did you want to say:  "too many objects" or "to match loaded object" ?
And what does it mean:
   "so final efforts to do a linear search less then sort and do binary 
serach" ?

Did you want to say something like:
   "do a linear search instead of binary search"


agent/src/share/classes/sun/jvm/hotspot/utilities/soql/sa.js

   nit: Remove redundant else at line 896, it saves the extra indent.

   nit: More simplifications to consider:

     function closestSymbolFor(addr) {
        var dso = (sa.cdbg == null) ? null : 
sa.cdbg.loadObjectContainingPC(addr);
        return (dso == null) ? null : dso.closestSymbolToPC(addr);
     }

     function loadObjectContainingPC(addr) {
         // sa.cdbg == null: no CDebugger support, return null
         return (sa.cdbg == null) ? null : 
sa.cdbg.loadObjectContainingPC(addr);
     }


Thanks,
Serguei

On 1/29/14 6:17 AM, Dmitry Samersoff wrote:
> Hi Everyone,
>
> Please review the fix.
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-7127191/webrev.02/
>
> -Dmitry
>
> On 2014-01-20 20:00, Dmitry Samersoff wrote:
>> Hi Everyone,
>>
>> Please review the fix.
>>
>> http://cr.openjdk.java.net/~dsamersoff/JDK-7127191/webrev.01/
>>
>> This fix doesn't solve all problems with symbol lookup in SA, but
>> address the problem mentioned in bug reports.
>>
>> 1. I change algorithm of pathmap_open. Now, it tries to find library
>> hardly.
>>
>> 2. I decided not to fix broken binary search in loadObjectContainingPC,
>> because with less then 20 DSO's we typically have here performance of
>> just linear search is approx the same as load, sort, convert to array
>> and do binary search.
>>
>> -Dmitry
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20140130/f8162991/attachment.html