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, coleen.phillim...@oracle.com 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, coleen.phillim...@oracle.com 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.
