On Fri, 30 Jun 2023 22:23:19 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>> Yi Yang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> memory leak > > I think commit "use HandshakeClosure instead of VMOperation" is a wrong way > to go. > It restricts use of parallel dumping only to attach case. > I have pending changes in heap dumper to support virtual threads and I'm > going switch heap dumper to always use DumpMerger. Hi @alexmenkov, > It restricts use of parallel dumping only to attach case. I'm not sure what you mean. Using handshake won't limit it to only attach cases; it can also be used in other cases like HeapDumpBeforeFullGC. The only difference is that previously, VMThread merged the dump files, and now it's the Attach listener that merges them. For the file merging process, my candidate options are as follows: 1. Execute with VMThread outside the safepoint, which will block VMThread from executing other vmoperations. 2. Execute with Attach listener thread outside the safepoint, which will block Attach listener from processing requests like jcmd/jstack. 3. Create a new temporary thread to execute the file merging outside the safepoint, which will have some resource consumption. I don't have a strong motivation to use 2, and 3 may be a good solution with the availability of virtual threads. If you have any concerns, we can consider using the most conservative option 1 to simplify the review process, and then optimize the file merging process in a follow-up patch. ------------- PR Comment: https://git.openjdk.org/jdk/pull/13667#issuecomment-1617115036
