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