RFR JDK-8059510 Compact symbol table layout inside shared archive

[Jiangli Zhou]

Thanks for looking into this, Staffan!

Jiangli

On 10/29/2014 04:47 AM, Staffan Larsen wrote:
> The diagnostic command-related changes look good!
>
> Thanks,
> /Staffan
>
> On 28 okt 2014, at 23:54, Jiangli Zhou <jiangli.zhou at oracle.com> wrote:
>
>> Hi Staffan,
>>
>> Here is updated webrev: http://cr.openjdk.java.net/~jiangli/8059510/webrev.05/. The permission changes for the new commands are in http://cr.openjdk.java.net/~jiangli/8059510/webrev.05/src/share/vm/classfile/compactHashtable.hpp.html.
>>
>> Thanks,
>> Jiangli
>>
>> On 10/28/2014 09:34 AM, Jiangli Zhou wrote:
>>> That's good to know! Thanks, Staffan. I'll set the permission for the new commands.
>>>
>>> Thanks,
>>> Jiangli
>>>
>>> On 10/28/2014 12:11 AM, Staffan Larsen wrote:
>>>> Thanks for doing these changes!
>>>>
>>>> I see one more thing that is missing from the Diagnostic Commands. Because these commands can potentially expose sensitive data, they should be protected from remote execution by anyone. Only authenticated users with the correct permissions should be able to run them. To achieve that, you simply have to override the permission() method in Dcmd like this:
>>>>
>>>>    static const JavaPermission permission() {
>>>>      JavaPermission p = {"java.lang.management.ManagementPermission",
>>>>                          "monitor", NULL};
>>>>      return p;
>>>>    }
>>>>
>>>> Thanks,
>>>> /Staffan
>>>>
>>>> On 28 okt 2014, at 01:21, Jiangli Zhou <jiangli.zhou at oracle.com> wrote:
>>>>
>>>>> Hi Staffan,
>>>>>
>>>>> I moved the symboltable/stringtable dumping implementations to symbolTable.cpp and stringTable.cpp. The SymboltableDCmd and StringtableDCmd class definitions are placed in the new compactHashTable.hpp. I also removed the -XX:+UnlockDiagnosticVMOptions requirement for the VM.symboltable and VM.stringtable options based on our separate discussion.
>>>>>
>>>>> http://cr.openjdk.java.net/~jiangli/8059510/webrev.04/
>>>>>
>>>>> Thanks for the suggestions,
>>>>>
>>>>> Jiangli
>>>>>
>>>>> On 10/23/2014 09:45 AM, Jiangli Zhou wrote:
>>>>>> Hi Staffan,
>>>>>>
>>>>>> No problem, the review request is still open. :) Thanks for the suggestion. I'll try to incorporate it.
>>>>>>
>>>>>> Thanks!
>>>>>> Jiangli
>>>>>>
>>>>>> On 10/22/2014 11:15 PM, Staffan Larsen wrote:
>>>>>>> Jiangli,
>>>>>>>
>>>>>>> Sorry for being late to the review. I would suggest moving the diagnostic command implementation from diagnosticCommand.cpp/hpp to symbolTable.cpp/hpp. If all dcmds are implemented in diagnosticCommand.cpp, then that file will grow to become quite large, so I would prefer having the commands close to the code they interact with (there are probably a couple more that should be moved). The only problem you may run into is to find a good place to call DCmdFactory::register_DCmdFactory(), but that can be done anywhere during the initialization of the VM.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> /Staffan
>>>>>>>
>>>>>>>
>>>>>>> On 22 okt 2014, at 02:09, Jiangli Zhou <jiangli.zhou at oracle.com> wrote:
>>>>>>>
>>>>>>>> Hi Gerard,
>>>>>>>>
>>>>>>>> Please see the updated webrev: http://cr.openjdk.java.net/~jiangli/8059510/webrev.03/.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Jiangli
>>>>>>>>
>>>>>>>> On 10/21/2014 02:57 PM, Jiangli Zhou wrote:
>>>>>>>>> Hi Gerard,
>>>>>>>>>
>>>>>>>>> On 10/21/2014 02:32 PM, Gerard Ziemski wrote:
>>>>>>>>>> On 10/21/2014 1:45 PM, Jiangli Zhou wrote:
>>>>>>>>>>> There are pros and cons for both dynamically and statically allocating the _shared_table. Dynamically allocating the _shared_table as you suggested adds overhead of the memory allocation and the overhead of the extra NULL check when the _shared_table is in use. When doing optimization, we try to have the least overhead for the fast case, so we could make the fast case faster (that's when CDS is used).
>>>>>>>>>>>
>>>>>>>>>>> Maybe we could make _shared_table.lookup() inlined and reduce the overhead of the function call when CDS is not used.
>>>>>>>>>> I think that would be OK, though it will require a reader of the code to follow the call through to find out it's cheap NOP when CDS is off, unless we add a nice comment here?
>>>>>>>>> I'll add some comments. Thanks for the suggestion.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Jiangli
>>>>>>>>>
>>>>>>>>>> cheers
>>>>>>>>>>