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.

[Jini George]

Gentle reminder!

Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.

[Ioi Lam]

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

Hi Jini,
The changes looks good to me.
Have you tested this on MacOS? CDS heap support is also enabled on
     #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
   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

Gentle reminder!
Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.

[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.

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

Post by Ioi Lam
Hi Jini,
The changes looks good to me.
Have you tested this on MacOS? CDS heap support is also enabled on
     #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
   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

Post by Ioi Lam
Gentle reminder!
Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.

[Jini George]

Gentle reminder.

Thanks!
Jini.

Post by 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
* 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.

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

Post by Ioi Lam
Hi Jini,
The changes looks good to me.
Have you tested this on MacOS? CDS heap support is also enabled on
     #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
   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

Post by Ioi Lam
Gentle reminder!
Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.

[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

Post by Jini George
Gentle reminder.
Thanks!
Jini.

Post by 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
* 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.

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

Post by Ioi Lam
Hi Jini,
The changes looks good to me.
Have you tested this on MacOS? CDS heap support is also enabled on
     #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
   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

Post by Ioi Lam
Gentle reminder!
Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.

[Kevin Walls]

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

Post by 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
* 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.

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

Post by Ioi Lam
Hi Jini,
The changes looks good to me.
Have you tested this on MacOS? CDS heap support is also enabled on
     #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
   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

Post by Ioi Lam
Gentle reminder!
Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.

[Ioi Lam]

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

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

Post by 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
* 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.

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

Post by Ioi Lam
Hi Jini,
The changes looks good to me.
Have you tested this on MacOS? CDS heap support is also enabled on
     #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
   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

Post by Ioi Lam
Gentle reminder!
Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.

[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.

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

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

Post by 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
* 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.

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

Post by Ioi Lam
Hi Jini,
The changes looks good to me.
Have you tested this on MacOS? CDS heap support is also enabled on
     #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
   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

Post by Ioi Lam
Gentle reminder!
Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.

[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.

Post by 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.

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

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

Post by 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
* 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.

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

Post by Ioi Lam
Hi Jini,
The changes looks good to me.
Have you tested this on MacOS? CDS heap support is also enabled on
     #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
   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

Post by Ioi Lam
Gentle reminder!
Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.

[Ioi Lam]

Hi Jini,

These changes look good to me. Thanks!

- Ioi

Post by Jini George
http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
* 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.

Post by 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.

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

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

Post by 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
* 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.

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

Post by Ioi Lam
Hi Jini,
The changes looks good to me.
Have you tested this on MacOS? CDS heap support is also enabled on
     #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
   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

Post by Ioi Lam
Gentle reminder!
Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.

[Jini George]

Thank you very much, Ioi!

- Jini.

Post by Ioi Lam
Hi Jini,
These changes look good to me. Thanks!
- Ioi

Post by Jini George
http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
* 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.

Post by 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.

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

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

Post by 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
* 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.

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

Post by Ioi Lam
Hi Jini,
The changes looks good to me.
Have you tested this on MacOS? CDS heap support is also enabled on
     #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
   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

Post by Ioi Lam
Gentle reminder!
Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.

[Kevin Walls]

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

Thanks
Kevin

Post by Jini George
Thank you very much, Ioi!
- Jini.

Post by Ioi Lam
Hi Jini,
These changes look good to me. Thanks!
- Ioi

Post by Jini George
http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
* 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.

Post by 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.

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

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

Post by 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
* 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.

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

Post by Ioi Lam
Hi Jini,
The changes looks good to me.
Have you tested this on MacOS? CDS heap support is also enabled on
     #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
   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

Post by Ioi Lam
Gentle reminder!
Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.

[Jini George]

Thank you very much, Kevin!

- Jini.

Post by Kevin Walls
Thanks for adding the flag Jini, all looks good to me too,
Thanks
Kevin

Post by Jini George
Thank you very much, Ioi!
- Jini.

Post by Ioi Lam
Hi Jini,
These changes look good to me. Thanks!
- Ioi

Post by Jini George
http://cr.openjdk.java.net/~jgeorge/8200613/webrev.02/index.html
* 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.

Post by 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.

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

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

Post by 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
* 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.

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

Post by Ioi Lam
Hi Jini,
The changes looks good to me.
Have you tested this on MacOS? CDS heap support is also enabled on
     #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
   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

Post by Ioi Lam
Gentle reminder!
Thanks,
- Jini

Post by Jini George
Hello!
[Including runtime-dev since the changes are in runtime code]
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.