RFR: (S): JDK-8200613: SA: jstack throws UnmappedAddressException with a CDS core file

Fri Nov 9 11:29:25 UTC 2018

Hi Jini,

Looks good to me.  It might be a significant increase in size of _some_ 
core files, but so many core files we see are much larger, in 
gigabytes++ of course, so the CDS data size should not be such a 
significant increase on (I think) most files.

The flexibiity of always having the CDS data there is very significant.  
A core file should ideally be usable, without additionally requiring the 
CDS archive from the machine.  That additional human round-trip upload 
request on every transmitted core that needs investigating, seems like a 
less efficient route...).

Is there an opt-out?  It's conditional on UseSharedSpaces but could 
there be a flag to disable, in case we see crashes with gigabytes of 
private mappings that we really don't want to retain (the user would 
have to know to set a flag, to disable the new coredump filter ahead of 
time).

Thanks!
Kevin

On 29/10/2018 06:02, Jini George wrote:
> Thank you very much, Ioi, for looking into this, and the clarification 
> offline. My bad, I had missed the earlier mail from you. :-( My 
> responses below.
>
> Yes, I had tested this on MacOS. The issue does not exist on MacOS 
> since the file backed private mmap()-ed regions get dumped into the 
> MacOS corefiles by default.
>
> The corefile sizes on Linux do increase due to this change. And the 
> increase would also include any file mapped using NIO with 
> MapMode.PRIVATE. The typical corefile size increase with this change 
> would include the following components at a high level:
>
> * Any NIO file mapping with MapMode.PRIVATE.
> * Any file mmap()-ed by any native library with MAP_PRIVATE.
> * The read only CDS regions (ro and od): Of the order of a few MB.
> * The shared strings CDS region. (typically less than 1 MB).
> * 2 MB per native shared library (regions with ---p permissions mapped 
> by the dynamic linker for better alignment and for keeping libraries 
> efficiently shareable).
> * The JDK 'modules' file. (About 140 MB).
>
> So, without including any NIO mapping, I typically see around 250-300 
> MB increase in the corefile sizes. I agree that the size increase 
> could be a cause for concern, but for FWIW, these privately mapped 
> files get dumped into the corefile for MacOS too. And the corefile 
> sizes for the same program on MacOS are way larger (of the order of a 
> few GB as against about 300 MB on Linux (without the change)).
>
> The advantage of fixing this by modifying the coredump_filter v/s 
> doing it in SA (by reading in more sections of the shared archive 
> file) is that this would benefit other debuggers like gdb also. (And 
> reduces the dependence on having the shared archive file being 
> available at the time of debugging). If folks still think this is a 
> cause for concern, I could make modifications to fix this by reading 
> in the regions from the shared archive file in the SA code. I also 
> wonder if it is worth taking a relook at the mapping types of the 
> various CDS regions also.
>
> Thank you,
> Jini.
>
> On 10/22/2018 10:27 AM, Ioi Lam wrote:
>> Hi Jini,
>>
>> Did you see my earlier reply? I might have sent it out during the 
>> mail server outage days :-(
>>
>> http://openjdk.5641.n7.nabble.com/RFR-S-JDK-8200613-SA-jstack-throws-UnmappedAddressException-with-a-CDS-core-file-td352681.html#a353026 
>>
>>
>> Here was my reply again:
>>
>>> Hi Jini,
>>>
>>> The changes looks good to me.
>>>
>>> Have you tested this on MacOS? CDS heap support is also enabled on
>>> MacOS. See macros.hpp:
>>>
>>>      #if INCLUDE_CDS && INCLUDE_G1GC && defined(_LP64) && 
>>> !defined(_WINDOWS)
>>>      #define INCLUDE_CDS_JAVA_HEAP 1
>>>
>>> Also, besides CDS, do we know how often other files will be mmaped with
>>> MAP_PRIVATE? Will users get huge core files because CDS is enabled? In
>>> JDK 12, CDS will be enabled by default (see JDK-8202951), so all users
>>> will be affected by the following line:
>>>
>>>    if (UseSharedSpaces) {
>>>      set_coredump_filter(FILE_BACKED_PVT_BIT);
>>>    }
>>>
>>> Maybe you can run an big app such as Eclipse, trigger a core dump, and
>>> compare the size of the core file before/after this change?
>>>
>>> Thanks
>>>
>>> - Ioi 
>>
>>
>>
>>
>>
>>
>> Thanks
>>
>> - Ioi
>>
>>
>> On 10/21/18 8:58 PM, Jini George wrote:
>>> Gentle reminder!
>>>
>>> Thanks,
>>> - Jini
>>>
>>> On 10/9/2018 11:31 AM, Jini George wrote:
>>>> Hello!
>>>>
>>>> [Including runtime-dev since the changes are in runtime code]
>>>>
>>>> Requesting reviews for:
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8200613/webrev.01/
>>>> BugID: https://bugs.openjdk.java.net/browse/JDK-8200613
>>>>
>>>> Issue: jhsdb jstack would throw an UnmappedAddressException with a 
>>>> core file generated from a CDS enabled java process. This is seen 
>>>> only with Linux and with G1GC, while trying to read in data from 
>>>> the shared strings region (the closed archive heap space). This 
>>>> region (which is a file backed private memory region) is not dumped 
>>>> into the corefile for Linux. This, being a heap region (and 
>>>> therefore being a read-write region) is also not read in from the 
>>>> classes.jsa file in SA since only the read only regions are read in 
>>>> while processing the core file. (The expectation being that the 
>>>> read write regions are in the core file).
>>>>
>>>> Proposed solution: The proposed solution is to have the 
>>>> coredump_filter value corresponding to the CDS process to include 
>>>> bit 2 (file-backed private memory), so that the file-backed private 
>>>> memory region also gets dumped into the corefile. The proposed fix 
>>>> is in src/hotspot/os/linux/os_linux.cpp.
>>>>
>>>> Thanks,
>>>> Jini.
>>>>
>>