Your changes look good to me overall, Yasumasa. A couple of points though.

* Since you are making modifications to have DumpPrivateMappingsInCore independent of CDS, in os::abort(), you would also need change the following lines:
1361 #if INCLUDE_CDS
1362     if (UseSharedSpaces && DumpPrivateMappingsInCore) {
1363       ClassLoader::close_jrt_image();
1364     }
1365 #endif

to

1361
1362     if (DumpPrivateMappingsInCore) {
1363       ClassLoader::close_jrt_image();
1364     }

But this might cause the zero build to fail due to the call to ClassLoader::close_jrt_image() (https://bugs.openjdk.java.net/browse/JDK-8215342). So, it might be better to guard the above lines with #ifndef ZERO to have:

1361 #ifndef ZERO
1362     if (DumpPrivateMappingsInCore) {
1363       ClassLoader::close_jrt_image();
1364     }
1365 #endif


* Also, one nit:

http://cr.openjdk.java.net/~ysuenaga/JDK-8219414/webrev.00/src/hotspot/os/linux/os_linux.cpp.html

- Could you please remove or modify the comment at line 3436 since set_coredump_filter() is not restricted to include only largepages now?

Thank you!
Jini.


On 2/21/2019 7:43 AM, Yasumasa Suenaga wrote:
Thanks Chris!


Yasumasa


2019年2月21日(木) 11:08 Chris Plummer <chris.plum...@oracle.com <mailto:chris.plum...@oracle.com>>:

    Ok. The changes look fine to me then.

    thanks,

    Chris

    On 2/20/19 6:02 PM, Yasumasa Suenaga wrote:
     > Hi Chris,
     >
     > I grep'ed with "MAP_SHARED" on jdk/src.
     > Shared memory is used in ZGC (ZBackingFile and
    ZPhysicalMemoryBacking)
     > and in FileChannel::map at least.
     >
     > I think we have memory footprint concern about them, but shared
    memory
     > should be dumped.
     > If we did not get them, we cannot analyze ZGC related behavior and
     > other code which uses shared memory.
     > (We can pass FD to os::reserve_memory - it will use shared memory if
     > FD is passed.)
     >
     > Thus I want to introduce `DumpSharedMappingsInCore` for dumping
    shared
     > memory and set it to true by default.
     >
     >
     > Thanks,
     >
     > Yasumasa
     >
     >
     > 2019年2月21日(木) 3:10 Chris Plummer <chris.plum...@oracle.com
    <mailto:chris.plum...@oracle.com>>:
     >> [adding runtime]
     >>
     >> Hi Yasumasa,
     >>
     >> Overall looks good. Just a couple of questions.
     >>
     >> Do we have the same footprint concerns with the shared mappings
    as we
     >> did with the private mappings? If not, possibly it doesn't need an
     >> option and should always be enabled.
     >>
     >> thanks,
     >>
     >> Chris
     >>
     >> On 2/20/19 12:03 AM, Yasumasa Suenaga wrote:
     >>> Hi all,
     >>>
     >>> Please review this webrev:
     >>>
     >>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8219414
     >>>     webrev:
    http://cr.openjdk.java.net/~ysuenaga/JDK-8219414/webrev.00/
     >>>
     >>> I tried to get PerfCounter values via `jhsdb jsnap` from core image
     >>> which is generated by `gcore` (provided by GDB), but I encountered
     >>> UnmappedAddressException.
     >>>
     >>> It is caused by `generate-core-file` on GDB regards
    `coredump_filter` on procfs.
     >>>
     >>> https://sourceware.org/gdb/onlinedocs/gdb/Core-File-Generation.html
     >>>
     >>> JDK-8200613 introduced `DumpPrivateMappingsInCore` for CDS. I
    want to
     >>> introduce `DumpSharedMappingsInCore` for shared memory mapping.
     >>>
     >>> Currently `DumpPrivateMappingsInCore` affects when
    `UseSharedSpaces` is enabled.
     >>> I want `DumpPrivateMappingsInCore` to affect independently in this
     >>> change because file-backed private memory which is not CDS might be
     >>> useful in the future.
     >>>
     >>>
     >>> Thanks,
     >>>
     >>> Yasumasa
     >>


Reply via email to