Looks good to me, Yasumasa. Thanks! -Jini.
On 2/21/2019 11:45 AM, Yasumasa Suenaga wrote:
Hi Jini, Thank you for your comment. I uploaded new webrev. Could you review again? http://cr.openjdk.java.net/~ysuenaga/JDK-8219414/webrev.01/ diff from webrev.00 is here: http://hg.openjdk.java.net/jdk/submit/rev/bb58c9d41381 Thanks, Yasumasa 2019年2月21日(木) 12:30 Jini George <jini.geo...@oracle.com>: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 >>
