RFR: JDK-8268893: jcmd to trim the glibc heap [v2]

[Thomas Stuefe]

On Mon, 21 Jun 2021 08:25:29 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Feedback Severin; renamed query function
>
> src/hotspot/os/linux/os_linux.cpp line 2142:
> 
>> 2140: bool os::Linux::query_process_memory_info(os::Linux::meminfo_t* info) {
>> 2141:   FILE* f = ::fopen("/proc/self/status", "r");
>> 2142:   const int num_values = 8;
> 
> Refactoring of `os::Linux::print_process_memory_info` looks good. But now that you've already created `os::Linux::meminfo_t` anyway I'd suggest to make `num_values == sizeof(os::Linux::meminfo_t)/sizeof(ssize_t)`.

Okay, makes sense. I was a bit apprehensive since that relies on the internal layout of this structure. But OTOH if that number is wrong, the worst that happens is that we, each time, parse through the whole of /proc/self/status instead of stopping early when all data are found. Which may happen anyway since not all kernels support all of these data.

> src/hotspot/share/services/diagnosticCommand.cpp line 99:
> 
>> 97:   DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<HeapInfoDCmd>(full_export, true, false));
>> 98:   DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<FinalizerInfoDCmd>(full_export, true, false));
>> 99:   LINUX_ONLY(DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<TrimCLibcHeapDCmd>(full_export, true, false));)
> 
> Not sure if the commands are really sorted and if order is important here. Otherwise you could add the new command a few lines later where there already exists an `#ifdef LINUX` section:
> 
> 
> #ifdef LINUX
>   DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<PerfMapDCmd>(full_export, true, false));
>   DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<TrimCLibcHeapDCmd>(full_export, true, false));
> #endif // LINUX

I don't think order matters, and grouping linux specific commands looks better.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4510