JOL Hotspot SA Support and Compressed References Implementation

[Aleksey Shipilev]

On 01/12/2015 08:03 PM, Serkan ÖZAL wrote:
>> On 01/06/2015 07:27 PM, Serkan ÖZAL wrote:
>> General things:
>>   
> Thanks, I will fix all of the issues as soon as possible and will send
> new patch.

Cool. I was so into reviewing the patch, and I forgot to say thank you
for this work. It sure one of the major improvements to JOL.

>>  * Is there a way to try and attach without super-user privileges first,
>> and the fall-back to "sudo" on failure? Asking for user password
>> interactively is asking for trouble, especially for library users.
>> Therefore, we are better off trying to attach automatically, and ask for
>> the password if that fails *and* some user property is set.
>>   
> OK. I will work on getting password interactively. In addition, what
> about getting password also as VM argument ? WDYT ?

No, sorry, do not get passwords interactively.

What I meant was this. Try to attach without superuser privileges. If it
fails, print the warning message, check the user property (e.g.
"jol.tryWithSudo"), and try the same command with "sudo". Let sudo do
the interactive part.

The point is to have completely non-interactive thing in the default
mode. If users accept to get the interactive sudo (as communicated by
the property), then do all the madness.

> "HotspotSAContext" is wrapper class to hold common necessary objects for
> Hotspot Serviceability Agent API Usage.
> It is passed to HS_SA_Processor's "process" method as parameter and
> designed for no-change on signature of this method since other objects
> may be added to context at the next versions.

Okay. Would it be simpler to drop now, and re-add later if actually
needed (which may never happen)? Unused code contributes to cruft.

>>  * Naming: OOP_SIZE vs KLASS_PTR_SIZE. Should be KLASS_OOP_SIZE?
>>   
> "KLASS_PTR" term comes from Hotspot SA codes, not my choice. So should I
> update as "KLASS_OOP_SIZE" ?

Yes, let it be KLASS_OOP_SIZE.

>>  * I fail to understand these comments: " // For backward compatibility,
>> OOP compressed reference mode can be used as default compressed
>> reference mode". Maybe you should provide a paragraph explaining what's
>> going on?
>>   
> I mean that, there is two different compressed references (OOP and
> Klass) information since Java 8. For Java 6 and 7, there is only one
> (OOP) and this one is used both OOP and Klass.
> So I assume compressed-oop information as compressed-reference
> information for users of  "USE_COMPRESSED_REFS" and
> "COMPRESSED_REF_SHIFT" properties.
> I hope this time it is more clear :)

Okay, you need to spell this out in comment block.

Thanks,
-Aleksey.