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

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Jini George
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.

Reply | Threaded
Open this post in threaded view
|

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

Ioi Lam
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


if FILE_BACKED_PVT_BIT is set,


On 10/8/18 11:01 PM, 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.
>

Reply | Threaded
Open this post in threaded view
|

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

Ioi Lam
In reply to this post by Jini George
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.
>>

Reply | Threaded
Open this post in threaded view
|

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

Jini George
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.
>>>
>
Reply | Threaded
Open this post in threaded view
|

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

Jini George
Gentle reminder.

Thanks!
Jini.

On 10/29/2018 11:32 AM, 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.
>>>>
>>
Reply | Threaded
Open this post in threaded view
|

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

Chris Plummer
Hi Jini,

I would counter your macos footprint argument by saying that macos
already has huge core files, so the % increase caused by your change is
small, whereas your indication is that on linux (a platform we tend to
care about debugging on much more) it is doubling the core file size.

thanks,

Chris

On 11/6/18 10:40 AM, Jini George wrote:

> Gentle reminder.
>
> Thanks!
> Jini.
>
> On 10/29/2018 11:32 AM, 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.
>>>>>
>>>


Reply | Threaded
Open this post in threaded view
|

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

KEVIN WALLS
In reply to this post by Jini George
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.
>>>>
>>

Reply | Threaded
Open this post in threaded view
|

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

Jini George
Thank you very much, Chris, Kevin and Ioi for your comments!

I will send another webrev with this change enabled under an opt-out
flag, as you suggest, and would look at unmapping the JDK modules file
and if possible, the NIO mapped files too in the signal handler.

Thanks a bunch,
Jini.

On 11/9/2018 11:53 PM, Ioi Lam wrote:

> Hi Jini,
>
> Thanks for investigating the size expansion issue.
>
> I agree that the size increase is worth it. Even when not using SA, if
> we open the core file inside GDB, we cannot read certain sections in the
> CDS archive (such as the RO section and strings sections). That would
> make debugging difficult. So I am in favor of this change.
>
> For the JDK modules file, maybe we can unmap it in the signal handler,
> before going ahead with the core dump? I think it's hardly needed for
> debugging purposes. (Perhaps we can also do the same for the NIO mapped
> files?)
>
> A opt-flag as suggested by Kevin is a good idea.
>
> Thanks
>
> - Ioi
>
> On 11/9/18 3:29 AM, Kevin Walls wrote:
>> 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.
>>>>>>
>>>>
>>
>
Reply | Threaded
Open this post in threaded view
|

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

Jini George
I have the revised webrev here:

http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html

The extra changes here are to:

* Introduce a new option DumpPrivateMappingsInCore to control the
dumping of the file backed private regions into the corefile.

* Close the modules file before dumping core in os::abort(). Currently,
there is a small bug (https://bugs.openjdk.java.net/browse/JDK-8215026)
which prevents the closure of the image file in unmapping the entire file.

I plan to take up the unmapping of NIO MapMode.PRIVATE files as a
separate task (https://bugs.openjdk.java.net/browse/JDK-8215027) since
this seems a bit involved.

Thanks a bunch,
Jini.

On 11/12/2018 10:26 AM, Jini George wrote:

> Thank you very much, Chris, Kevin and Ioi for your comments!
>
> I will send another webrev with this change enabled under an opt-out
> flag, as you suggest, and would look at unmapping the JDK modules file
> and if possible, the NIO mapped files too in the signal handler.
>
> Thanks a bunch,
> Jini.
>
> On 11/9/2018 11:53 PM, Ioi Lam wrote:
>> Hi Jini,
>>
>> Thanks for investigating the size expansion issue.
>>
>> I agree that the size increase is worth it. Even when not using SA, if
>> we open the core file inside GDB, we cannot read certain sections in
>> the CDS archive (such as the RO section and strings sections). That
>> would make debugging difficult. So I am in favor of this change.
>>
>> For the JDK modules file, maybe we can unmap it in the signal handler,
>> before going ahead with the core dump? I think it's hardly needed for
>> debugging purposes. (Perhaps we can also do the same for the NIO
>> mapped files?)
>>
>> A opt-flag as suggested by Kevin is a good idea.
>>
>> Thanks
>>
>> - Ioi
>>
>> On 11/9/18 3:29 AM, Kevin Walls wrote:
>>> 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.
>>>>>>>
>>>>>
>>>
>>
Reply | Threaded
Open this post in threaded view
|

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

Coleen Phillimore-2

Hi Jini,  We were just talking about this new option.  If someone gets a
crash, I don't think they're going to run their application again with
-XX:-DumpPrivateMappingsInCore in the case of the core file being too
large.  So I don't know how generally useful this option is. I think it
would be better to not add it and set the bit to include the mappings
unconditionally.

How much larger is this core file (we had trouble understanding from the
bug)?  If you need the mappings to understand and use the SA tools on
it, we want to have them.

Thanks,
Coleen


On 12/7/18 2:22 PM, Jini George wrote:

> I have the revised webrev here:
>
> http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
>
> The extra changes here are to:
>
> * Introduce a new option DumpPrivateMappingsInCore to control the
> dumping of the file backed private regions into the corefile.
>
> * Close the modules file before dumping core in os::abort().
> Currently, there is a small bug
> (https://bugs.openjdk.java.net/browse/JDK-8215026) which prevents the
> closure of the image file in unmapping the entire file.
>
> I plan to take up the unmapping of NIO MapMode.PRIVATE files as a
> separate task (https://bugs.openjdk.java.net/browse/JDK-8215027) since
> this seems a bit involved.
>
> Thanks a bunch,
> Jini.
>
> On 11/12/2018 10:26 AM, Jini George wrote:
>> Thank you very much, Chris, Kevin and Ioi for your comments!
>>
>> I will send another webrev with this change enabled under an opt-out
>> flag, as you suggest, and would look at unmapping the JDK modules
>> file and if possible, the NIO mapped files too in the signal handler.
>>
>> Thanks a bunch,
>> Jini.
>>
>> On 11/9/2018 11:53 PM, Ioi Lam wrote:
>>> Hi Jini,
>>>
>>> Thanks for investigating the size expansion issue.
>>>
>>> I agree that the size increase is worth it. Even when not using SA,
>>> if we open the core file inside GDB, we cannot read certain sections
>>> in the CDS archive (such as the RO section and strings sections).
>>> That would make debugging difficult. So I am in favor of this change.
>>>
>>> For the JDK modules file, maybe we can unmap it in the signal
>>> handler, before going ahead with the core dump? I think it's hardly
>>> needed for debugging purposes. (Perhaps we can also do the same for
>>> the NIO mapped files?)
>>>
>>> A opt-flag as suggested by Kevin is a good idea.
>>>
>>> Thanks
>>>
>>> - Ioi
>>>
>>> On 11/9/18 3:29 AM, Kevin Walls wrote:
>>>> 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.
>>>>>>>>
>>>>>>
>>>>
>>>

Reply | Threaded
Open this post in threaded view
|

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

Ioi Lam
In reply to this post by Jini George
Hi Jini,

These changes look good to me. Thanks!

- Ioi


On 12/7/18 11:22 AM, Jini George wrote:

> I have the revised webrev here:
>
> http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
>
> The extra changes here are to:
>
> * Introduce a new option DumpPrivateMappingsInCore to control the
> dumping of the file backed private regions into the corefile.
>
> * Close the modules file before dumping core in os::abort().
> Currently, there is a small bug
> (https://bugs.openjdk.java.net/browse/JDK-8215026) which prevents the
> closure of the image file in unmapping the entire file.
>
> I plan to take up the unmapping of NIO MapMode.PRIVATE files as a
> separate task (https://bugs.openjdk.java.net/browse/JDK-8215027) since
> this seems a bit involved.
>
> Thanks a bunch,
> Jini.
>
> On 11/12/2018 10:26 AM, Jini George wrote:
>> Thank you very much, Chris, Kevin and Ioi for your comments!
>>
>> I will send another webrev with this change enabled under an opt-out
>> flag, as you suggest, and would look at unmapping the JDK modules
>> file and if possible, the NIO mapped files too in the signal handler.
>>
>> Thanks a bunch,
>> Jini.
>>
>> On 11/9/2018 11:53 PM, Ioi Lam wrote:
>>> Hi Jini,
>>>
>>> Thanks for investigating the size expansion issue.
>>>
>>> I agree that the size increase is worth it. Even when not using SA,
>>> if we open the core file inside GDB, we cannot read certain sections
>>> in the CDS archive (such as the RO section and strings sections).
>>> That would make debugging difficult. So I am in favor of this change.
>>>
>>> For the JDK modules file, maybe we can unmap it in the signal
>>> handler, before going ahead with the core dump? I think it's hardly
>>> needed for debugging purposes. (Perhaps we can also do the same for
>>> the NIO mapped files?)
>>>
>>> A opt-flag as suggested by Kevin is a good idea.
>>>
>>> Thanks
>>>
>>> - Ioi
>>>
>>> On 11/9/18 3:29 AM, Kevin Walls wrote:
>>>> 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.
>>>>>>>>
>>>>>>
>>>>
>>>

Reply | Threaded
Open this post in threaded view
|

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

Ioi Lam
In reply to this post by Coleen Phillimore-2
Hi Coleen,

I was one of the people who suggested the DumpPrivateMappingsInCore
flag. It's enabled by default, so by default all the contents of mmap'ed
files with MAP_PRIVATE will be saved to the core files.

The worry is, there may be some extreme cases where the JVM has mapped
very large files (with NIO or JNI, etc). For example, you could have a
100GB in-memory database. For those cases, if the user is experiencing
crashes, but they are unable to get a core dump (because it would be too
large), they can try running with -XX:-DumpPrivateMappingsInCore.

Thanks

- Ioi


On 12/7/18 12:03 PM, [hidden email] wrote:

>
> Hi Jini,  We were just talking about this new option.  If someone gets
> a crash, I don't think they're going to run their application again
> with -XX:-DumpPrivateMappingsInCore in the case of the core file being
> too large.  So I don't know how generally useful this option is. I
> think it would be better to not add it and set the bit to include the
> mappings unconditionally.
>
> How much larger is this core file (we had trouble understanding from
> the bug)?  If you need the mappings to understand and use the SA tools
> on it, we want to have them.
>
> Thanks,
> Coleen
>
>
> On 12/7/18 2:22 PM, Jini George wrote:
>> I have the revised webrev here:
>>
>> http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
>>
>> The extra changes here are to:
>>
>> * Introduce a new option DumpPrivateMappingsInCore to control the
>> dumping of the file backed private regions into the corefile.
>>
>> * Close the modules file before dumping core in os::abort().
>> Currently, there is a small bug
>> (https://bugs.openjdk.java.net/browse/JDK-8215026) which prevents the
>> closure of the image file in unmapping the entire file.
>>
>> I plan to take up the unmapping of NIO MapMode.PRIVATE files as a
>> separate task (https://bugs.openjdk.java.net/browse/JDK-8215027)
>> since this seems a bit involved.
>>
>> Thanks a bunch,
>> Jini.
>>
>> On 11/12/2018 10:26 AM, Jini George wrote:
>>> Thank you very much, Chris, Kevin and Ioi for your comments!
>>>
>>> I will send another webrev with this change enabled under an opt-out
>>> flag, as you suggest, and would look at unmapping the JDK modules
>>> file and if possible, the NIO mapped files too in the signal handler.
>>>
>>> Thanks a bunch,
>>> Jini.
>>>
>>> On 11/9/2018 11:53 PM, Ioi Lam wrote:
>>>> Hi Jini,
>>>>
>>>> Thanks for investigating the size expansion issue.
>>>>
>>>> I agree that the size increase is worth it. Even when not using SA,
>>>> if we open the core file inside GDB, we cannot read certain
>>>> sections in the CDS archive (such as the RO section and strings
>>>> sections). That would make debugging difficult. So I am in favor of
>>>> this change.
>>>>
>>>> For the JDK modules file, maybe we can unmap it in the signal
>>>> handler, before going ahead with the core dump? I think it's hardly
>>>> needed for debugging purposes. (Perhaps we can also do the same for
>>>> the NIO mapped files?)
>>>>
>>>> A opt-flag as suggested by Kevin is a good idea.
>>>>
>>>> Thanks
>>>>
>>>> - Ioi
>>>>
>>>> On 11/9/18 3:29 AM, Kevin Walls wrote:
>>>>> 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.
>>>>>>>>>
>>>>>>>
>>>>>
>>>>
>

Reply | Threaded
Open this post in threaded view
|

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

Jini George
In reply to this post by Ioi Lam
Thank you very much, Ioi!

- Jini.

On 12/10/2018 12:01 PM, Ioi Lam wrote:

> Hi Jini,
>
> These changes look good to me. Thanks!
>
> - Ioi
>
>
> On 12/7/18 11:22 AM, Jini George wrote:
>> I have the revised webrev here:
>>
>> http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
>>
>> The extra changes here are to:
>>
>> * Introduce a new option DumpPrivateMappingsInCore to control the
>> dumping of the file backed private regions into the corefile.
>>
>> * Close the modules file before dumping core in os::abort().
>> Currently, there is a small bug
>> (https://bugs.openjdk.java.net/browse/JDK-8215026) which prevents the
>> closure of the image file in unmapping the entire file.
>>
>> I plan to take up the unmapping of NIO MapMode.PRIVATE files as a
>> separate task (https://bugs.openjdk.java.net/browse/JDK-8215027) since
>> this seems a bit involved.
>>
>> Thanks a bunch,
>> Jini.
>>
>> On 11/12/2018 10:26 AM, Jini George wrote:
>>> Thank you very much, Chris, Kevin and Ioi for your comments!
>>>
>>> I will send another webrev with this change enabled under an opt-out
>>> flag, as you suggest, and would look at unmapping the JDK modules
>>> file and if possible, the NIO mapped files too in the signal handler.
>>>
>>> Thanks a bunch,
>>> Jini.
>>>
>>> On 11/9/2018 11:53 PM, Ioi Lam wrote:
>>>> Hi Jini,
>>>>
>>>> Thanks for investigating the size expansion issue.
>>>>
>>>> I agree that the size increase is worth it. Even when not using SA,
>>>> if we open the core file inside GDB, we cannot read certain sections
>>>> in the CDS archive (such as the RO section and strings sections).
>>>> That would make debugging difficult. So I am in favor of this change.
>>>>
>>>> For the JDK modules file, maybe we can unmap it in the signal
>>>> handler, before going ahead with the core dump? I think it's hardly
>>>> needed for debugging purposes. (Perhaps we can also do the same for
>>>> the NIO mapped files?)
>>>>
>>>> A opt-flag as suggested by Kevin is a good idea.
>>>>
>>>> Thanks
>>>>
>>>> - Ioi
>>>>
>>>> On 11/9/18 3:29 AM, Kevin Walls wrote:
>>>>> 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.
>>>>>>>>>
>>>>>>>
>>>>>
>>>>
>
Reply | Threaded
Open this post in threaded view
|

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

KEVIN WALLS
Thanks for adding the flag Jini, all looks good to me too,

Thanks
Kevin


On 10/12/2018 06:44, Jini George wrote:

> Thank you very much, Ioi!
>
> - Jini.
>
> On 12/10/2018 12:01 PM, Ioi Lam wrote:
>> Hi Jini,
>>
>> These changes look good to me. Thanks!
>>
>> - Ioi
>>
>>
>> On 12/7/18 11:22 AM, Jini George wrote:
>>> I have the revised webrev here:
>>>
>>> http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
>>>
>>> The extra changes here are to:
>>>
>>> * Introduce a new option DumpPrivateMappingsInCore to control the
>>> dumping of the file backed private regions into the corefile.
>>>
>>> * Close the modules file before dumping core in os::abort().
>>> Currently, there is a small bug
>>> (https://bugs.openjdk.java.net/browse/JDK-8215026) which prevents
>>> the closure of the image file in unmapping the entire file.
>>>
>>> I plan to take up the unmapping of NIO MapMode.PRIVATE files as a
>>> separate task (https://bugs.openjdk.java.net/browse/JDK-8215027)
>>> since this seems a bit involved.
>>>
>>> Thanks a bunch,
>>> Jini.
>>>
>>> On 11/12/2018 10:26 AM, Jini George wrote:
>>>> Thank you very much, Chris, Kevin and Ioi for your comments!
>>>>
>>>> I will send another webrev with this change enabled under an
>>>> opt-out flag, as you suggest, and would look at unmapping the JDK
>>>> modules file and if possible, the NIO mapped files too in the
>>>> signal handler.
>>>>
>>>> Thanks a bunch,
>>>> Jini.
>>>>
>>>> On 11/9/2018 11:53 PM, Ioi Lam wrote:
>>>>> Hi Jini,
>>>>>
>>>>> Thanks for investigating the size expansion issue.
>>>>>
>>>>> I agree that the size increase is worth it. Even when not using
>>>>> SA, if we open the core file inside GDB, we cannot read certain
>>>>> sections in the CDS archive (such as the RO section and strings
>>>>> sections). That would make debugging difficult. So I am in favor
>>>>> of this change.
>>>>>
>>>>> For the JDK modules file, maybe we can unmap it in the signal
>>>>> handler, before going ahead with the core dump? I think it's
>>>>> hardly needed for debugging purposes. (Perhaps we can also do the
>>>>> same for the NIO mapped files?)
>>>>>
>>>>> A opt-flag as suggested by Kevin is a good idea.
>>>>>
>>>>> Thanks
>>>>>
>>>>> - Ioi
>>>>>
>>>>> On 11/9/18 3:29 AM, Kevin Walls wrote:
>>>>>> 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.
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>

Reply | Threaded
Open this post in threaded view
|

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

Jini George
Thank you very much, Kevin!

- Jini.

On 12/10/2018 3:09 PM, Kevin Walls wrote:

> Thanks for adding the flag Jini, all looks good to me too,
>
> Thanks
> Kevin
>
>
> On 10/12/2018 06:44, Jini George wrote:
>> Thank you very much, Ioi!
>>
>> - Jini.
>>
>> On 12/10/2018 12:01 PM, Ioi Lam wrote:
>>> Hi Jini,
>>>
>>> These changes look good to me. Thanks!
>>>
>>> - Ioi
>>>
>>>
>>> On 12/7/18 11:22 AM, Jini George wrote:
>>>> I have the revised webrev here:
>>>>
>>>> http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
>>>>
>>>> The extra changes here are to:
>>>>
>>>> * Introduce a new option DumpPrivateMappingsInCore to control the
>>>> dumping of the file backed private regions into the corefile.
>>>>
>>>> * Close the modules file before dumping core in os::abort().
>>>> Currently, there is a small bug
>>>> (https://bugs.openjdk.java.net/browse/JDK-8215026) which prevents
>>>> the closure of the image file in unmapping the entire file.
>>>>
>>>> I plan to take up the unmapping of NIO MapMode.PRIVATE files as a
>>>> separate task (https://bugs.openjdk.java.net/browse/JDK-8215027)
>>>> since this seems a bit involved.
>>>>
>>>> Thanks a bunch,
>>>> Jini.
>>>>
>>>> On 11/12/2018 10:26 AM, Jini George wrote:
>>>>> Thank you very much, Chris, Kevin and Ioi for your comments!
>>>>>
>>>>> I will send another webrev with this change enabled under an
>>>>> opt-out flag, as you suggest, and would look at unmapping the JDK
>>>>> modules file and if possible, the NIO mapped files too in the
>>>>> signal handler.
>>>>>
>>>>> Thanks a bunch,
>>>>> Jini.
>>>>>
>>>>> On 11/9/2018 11:53 PM, Ioi Lam wrote:
>>>>>> Hi Jini,
>>>>>>
>>>>>> Thanks for investigating the size expansion issue.
>>>>>>
>>>>>> I agree that the size increase is worth it. Even when not using
>>>>>> SA, if we open the core file inside GDB, we cannot read certain
>>>>>> sections in the CDS archive (such as the RO section and strings
>>>>>> sections). That would make debugging difficult. So I am in favor
>>>>>> of this change.
>>>>>>
>>>>>> For the JDK modules file, maybe we can unmap it in the signal
>>>>>> handler, before going ahead with the core dump? I think it's
>>>>>> hardly needed for debugging purposes. (Perhaps we can also do the
>>>>>> same for the NIO mapped files?)
>>>>>>
>>>>>> A opt-flag as suggested by Kevin is a good idea.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> - Ioi
>>>>>>
>>>>>> On 11/9/18 3:29 AM, Kevin Walls wrote:
>>>>>>> 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.
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

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

Coleen Phillimore-2
In reply to this post by Ioi Lam

I see.  In this case, the flag should be diagnostic and not require a
CSR.  I suppose some documentation should be added so that sustaining
will know about the option in this case (Hi Kevin!)

thanks,
Coleen


On 12/10/18 1:38 AM, Ioi Lam wrote:

> Hi Coleen,
>
> I was one of the people who suggested the DumpPrivateMappingsInCore
> flag. It's enabled by default, so by default all the contents of
> mmap'ed files with MAP_PRIVATE will be saved to the core files.
>
> The worry is, there may be some extreme cases where the JVM has mapped
> very large files (with NIO or JNI, etc). For example, you could have a
> 100GB in-memory database. For those cases, if the user is experiencing
> crashes, but they are unable to get a core dump (because it would be
> too large), they can try running with -XX:-DumpPrivateMappingsInCore.
>
> Thanks
>
> - Ioi
>
>
> On 12/7/18 12:03 PM, [hidden email] wrote:
>>
>> Hi Jini,  We were just talking about this new option.  If someone
>> gets a crash, I don't think they're going to run their application
>> again with -XX:-DumpPrivateMappingsInCore in the case of the core
>> file being too large.  So I don't know how generally useful this
>> option is. I think it would be better to not add it and set the bit
>> to include the mappings unconditionally.
>>
>> How much larger is this core file (we had trouble understanding from
>> the bug)?  If you need the mappings to understand and use the SA
>> tools on it, we want to have them.
>>
>> Thanks,
>> Coleen
>>
>>
>> On 12/7/18 2:22 PM, Jini George wrote:
>>> I have the revised webrev here:
>>>
>>> http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
>>>
>>> The extra changes here are to:
>>>
>>> * Introduce a new option DumpPrivateMappingsInCore to control the
>>> dumping of the file backed private regions into the corefile.
>>>
>>> * Close the modules file before dumping core in os::abort().
>>> Currently, there is a small bug
>>> (https://bugs.openjdk.java.net/browse/JDK-8215026) which prevents
>>> the closure of the image file in unmapping the entire file.
>>>
>>> I plan to take up the unmapping of NIO MapMode.PRIVATE files as a
>>> separate task (https://bugs.openjdk.java.net/browse/JDK-8215027)
>>> since this seems a bit involved.
>>>
>>> Thanks a bunch,
>>> Jini.
>>>
>>> On 11/12/2018 10:26 AM, Jini George wrote:
>>>> Thank you very much, Chris, Kevin and Ioi for your comments!
>>>>
>>>> I will send another webrev with this change enabled under an
>>>> opt-out flag, as you suggest, and would look at unmapping the JDK
>>>> modules file and if possible, the NIO mapped files too in the
>>>> signal handler.
>>>>
>>>> Thanks a bunch,
>>>> Jini.
>>>>
>>>> On 11/9/2018 11:53 PM, Ioi Lam wrote:
>>>>> Hi Jini,
>>>>>
>>>>> Thanks for investigating the size expansion issue.
>>>>>
>>>>> I agree that the size increase is worth it. Even when not using
>>>>> SA, if we open the core file inside GDB, we cannot read certain
>>>>> sections in the CDS archive (such as the RO section and strings
>>>>> sections). That would make debugging difficult. So I am in favor
>>>>> of this change.
>>>>>
>>>>> For the JDK modules file, maybe we can unmap it in the signal
>>>>> handler, before going ahead with the core dump? I think it's
>>>>> hardly needed for debugging purposes. (Perhaps we can also do the
>>>>> same for the NIO mapped files?)
>>>>>
>>>>> A opt-flag as suggested by Kevin is a good idea.
>>>>>
>>>>> Thanks
>>>>>
>>>>> - Ioi
>>>>>
>>>>> On 11/9/18 3:29 AM, Kevin Walls wrote:
>>>>>> 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.
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

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

Jini George
Thank you very much, Coleen. I have converted the flag into a diagnostic
flag. The revised webrev is at:

http://cr.openjdk.java.net/~jgeorge/8200613/webrev.03/

A plain diff between this patch and the earlier one is at:

http://cr.openjdk.java.net/~jgeorge/8200613/incremental_diff

I will withdraw the associated CSR.

Thanks!
Jini.

On 12/10/2018 11:55 PM, [hidden email] wrote:

>
> I see.  In this case, the flag should be diagnostic and not require a
> CSR.  I suppose some documentation should be added so that sustaining
> will know about the option in this case (Hi Kevin!)
>
> thanks,
> Coleen
>
>
> On 12/10/18 1:38 AM, Ioi Lam wrote:
>> Hi Coleen,
>>
>> I was one of the people who suggested the DumpPrivateMappingsInCore
>> flag. It's enabled by default, so by default all the contents of
>> mmap'ed files with MAP_PRIVATE will be saved to the core files.
>>
>> The worry is, there may be some extreme cases where the JVM has mapped
>> very large files (with NIO or JNI, etc). For example, you could have a
>> 100GB in-memory database. For those cases, if the user is experiencing
>> crashes, but they are unable to get a core dump (because it would be
>> too large), they can try running with -XX:-DumpPrivateMappingsInCore.
>>
>> Thanks
>>
>> - Ioi
>>
>>
>> On 12/7/18 12:03 PM, [hidden email] wrote:
>>>
>>> Hi Jini,  We were just talking about this new option.  If someone
>>> gets a crash, I don't think they're going to run their application
>>> again with -XX:-DumpPrivateMappingsInCore in the case of the core
>>> file being too large.  So I don't know how generally useful this
>>> option is. I think it would be better to not add it and set the bit
>>> to include the mappings unconditionally.
>>>
>>> How much larger is this core file (we had trouble understanding from
>>> the bug)?  If you need the mappings to understand and use the SA
>>> tools on it, we want to have them.
>>>
>>> Thanks,
>>> Coleen
>>>
>>>
>>> On 12/7/18 2:22 PM, Jini George wrote:
>>>> I have the revised webrev here:
>>>>
>>>> http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
>>>>
>>>> The extra changes here are to:
>>>>
>>>> * Introduce a new option DumpPrivateMappingsInCore to control the
>>>> dumping of the file backed private regions into the corefile.
>>>>
>>>> * Close the modules file before dumping core in os::abort().
>>>> Currently, there is a small bug
>>>> (https://bugs.openjdk.java.net/browse/JDK-8215026) which prevents
>>>> the closure of the image file in unmapping the entire file.
>>>>
>>>> I plan to take up the unmapping of NIO MapMode.PRIVATE files as a
>>>> separate task (https://bugs.openjdk.java.net/browse/JDK-8215027)
>>>> since this seems a bit involved.
>>>>
>>>> Thanks a bunch,
>>>> Jini.
>>>>
>>>> On 11/12/2018 10:26 AM, Jini George wrote:
>>>>> Thank you very much, Chris, Kevin and Ioi for your comments!
>>>>>
>>>>> I will send another webrev with this change enabled under an
>>>>> opt-out flag, as you suggest, and would look at unmapping the JDK
>>>>> modules file and if possible, the NIO mapped files too in the
>>>>> signal handler.
>>>>>
>>>>> Thanks a bunch,
>>>>> Jini.
>>>>>
>>>>> On 11/9/2018 11:53 PM, Ioi Lam wrote:
>>>>>> Hi Jini,
>>>>>>
>>>>>> Thanks for investigating the size expansion issue.
>>>>>>
>>>>>> I agree that the size increase is worth it. Even when not using
>>>>>> SA, if we open the core file inside GDB, we cannot read certain
>>>>>> sections in the CDS archive (such as the RO section and strings
>>>>>> sections). That would make debugging difficult. So I am in favor
>>>>>> of this change.
>>>>>>
>>>>>> For the JDK modules file, maybe we can unmap it in the signal
>>>>>> handler, before going ahead with the core dump? I think it's
>>>>>> hardly needed for debugging purposes. (Perhaps we can also do the
>>>>>> same for the NIO mapped files?)
>>>>>>
>>>>>> A opt-flag as suggested by Kevin is a good idea.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> - Ioi
>>>>>>
>>>>>> On 11/9/18 3:29 AM, Kevin Walls wrote:
>>>>>>> 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.
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>
>>>
>>
>
Reply | Threaded
Open this post in threaded view
|

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

Jini George
Thank you, Coleen!

-Jini.

On 12/12/2018 12:58 AM, [hidden email] wrote:

>
>
> On 12/11/18 2:37 AM, Jini George wrote:
>> Thank you very much, Coleen. I have converted the flag into a
>> diagnostic flag. The revised webrev is at:
>>
>> http://cr.openjdk.java.net/~jgeorge/8200613/webrev.03/
>
> Looks great!
>>
>> A plain diff between this patch and the earlier one is at:
>>
>> http://cr.openjdk.java.net/~jgeorge/8200613/incremental_diff
>>
>> I will withdraw the associated CSR.
>
> Yes, please withdraw it.  Thank you!
>
> Coleen
>
>>
>> Thanks!
>> Jini.
>>
>> On 12/10/2018 11:55 PM, [hidden email] wrote:
>>>
>>> I see.  In this case, the flag should be diagnostic and not require a
>>> CSR.  I suppose some documentation should be added so that sustaining
>>> will know about the option in this case (Hi Kevin!)
>>>
>>> thanks,
>>> Coleen
>>>
>>>
>>> On 12/10/18 1:38 AM, Ioi Lam wrote:
>>>> Hi Coleen,
>>>>
>>>> I was one of the people who suggested the DumpPrivateMappingsInCore
>>>> flag. It's enabled by default, so by default all the contents of
>>>> mmap'ed files with MAP_PRIVATE will be saved to the core files.
>>>>
>>>> The worry is, there may be some extreme cases where the JVM has
>>>> mapped very large files (with NIO or JNI, etc). For example, you
>>>> could have a 100GB in-memory database. For those cases, if the user
>>>> is experiencing crashes, but they are unable to get a core dump
>>>> (because it would be too large), they can try running with
>>>> -XX:-DumpPrivateMappingsInCore.
>>>>
>>>> Thanks
>>>>
>>>> - Ioi
>>>>
>>>>
>>>> On 12/7/18 12:03 PM, [hidden email] wrote:
>>>>>
>>>>> Hi Jini,  We were just talking about this new option.  If someone
>>>>> gets a crash, I don't think they're going to run their application
>>>>> again with -XX:-DumpPrivateMappingsInCore in the case of the core
>>>>> file being too large.  So I don't know how generally useful this
>>>>> option is. I think it would be better to not add it and set the bit
>>>>> to include the mappings unconditionally.
>>>>>
>>>>> How much larger is this core file (we had trouble understanding
>>>>> from the bug)?  If you need the mappings to understand and use the
>>>>> SA tools on it, we want to have them.
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>>
>>>>>
>>>>> On 12/7/18 2:22 PM, Jini George wrote:
>>>>>> I have the revised webrev here:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
>>>>>>
>>>>>> The extra changes here are to:
>>>>>>
>>>>>> * Introduce a new option DumpPrivateMappingsInCore to control the
>>>>>> dumping of the file backed private regions into the corefile.
>>>>>>
>>>>>> * Close the modules file before dumping core in os::abort().
>>>>>> Currently, there is a small bug
>>>>>> (https://bugs.openjdk.java.net/browse/JDK-8215026) which prevents
>>>>>> the closure of the image file in unmapping the entire file.
>>>>>>
>>>>>> I plan to take up the unmapping of NIO MapMode.PRIVATE files as a
>>>>>> separate task (https://bugs.openjdk.java.net/browse/JDK-8215027)
>>>>>> since this seems a bit involved.
>>>>>>
>>>>>> Thanks a bunch,
>>>>>> Jini.
>>>>>>
>>>>>> On 11/12/2018 10:26 AM, Jini George wrote:
>>>>>>> Thank you very much, Chris, Kevin and Ioi for your comments!
>>>>>>>
>>>>>>> I will send another webrev with this change enabled under an
>>>>>>> opt-out flag, as you suggest, and would look at unmapping the JDK
>>>>>>> modules file and if possible, the NIO mapped files too in the
>>>>>>> signal handler.
>>>>>>>
>>>>>>> Thanks a bunch,
>>>>>>> Jini.
>>>>>>>
>>>>>>> On 11/9/2018 11:53 PM, Ioi Lam wrote:
>>>>>>>> Hi Jini,
>>>>>>>>
>>>>>>>> Thanks for investigating the size expansion issue.
>>>>>>>>
>>>>>>>> I agree that the size increase is worth it. Even when not using
>>>>>>>> SA, if we open the core file inside GDB, we cannot read certain
>>>>>>>> sections in the CDS archive (such as the RO section and strings
>>>>>>>> sections). That would make debugging difficult. So I am in favor
>>>>>>>> of this change.
>>>>>>>>
>>>>>>>> For the JDK modules file, maybe we can unmap it in the signal
>>>>>>>> handler, before going ahead with the core dump? I think it's
>>>>>>>> hardly needed for debugging purposes. (Perhaps we can also do
>>>>>>>> the same for the NIO mapped files?)
>>>>>>>>
>>>>>>>> A opt-flag as suggested by Kevin is a good idea.
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>>
>>>>>>>> - Ioi
>>>>>>>>
>>>>>>>> On 11/9/18 3:29 AM, Kevin Walls wrote:
>>>>>>>>> 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.
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>
>>>>
>>>
>