Re: [RFR]8215622: Add dump to file support for jmap histo

[serguei.spit...@oracle.com]

Dear Lin, On 2/11/19 17:49, 臧琳 wrote: Dear All, Thanks again for the comments. one question from me is that "jmap -dump" must accept an argument indicates location of the heap dump file. if there is no such argument, it fails. (it is meaningless to output the binary directly to stdout/stderr for me.) Does the option "format=b" tells us that the dump output can be in text format as well? The help tells nothing about what is the default format. I'd conclude that the default has to be text format if the option "format=b" is missed. Now, I'm looking at the option processing in the dump() method and see that option "format=b" is not really processed but just filtered out. It means that you are right, the only supported dumping format is binary. In this situation, I withdraw my suggestion to allow empty <dump-options>. At least, you do not need to care about it. However, there is still the suggestion from Paul to replace "and" with "or" in both dump-options and histo-options. so adding "jmap -dump" actually just output error message like "No dump file specified", rather than print the usage of JMap as implemented now. And this is also why there is no "-dump" or "-dump:" test in BasicJMapTest.java. I leave it to other people to decide what is better here. Another option is to add default file generated at canonical path for -dump. then I can add the unit test. which one do you think is more reasonable? It looks like a good idea. But I'm not sure about it yet. Let's see what others say. Moreover, one minor thing is that BasicJMapTest right now has test for "jmap -histo:" , not for "jmap -histo", so I think I need to add test for that. is this OK? Yes. Thanks, Serguei Cheers, Lin ________________________________________ From: Joseph D. Darcy <joe.da...@oracle.com> Sent: Tuesday, February 12, 2019 6:22:59 AM To: Hohensee, Paul; serguei.spit...@oracle.com; 臧琳; David Holmes; JC Beyler Cc: serviceability-dev@openjdk.java.net Subject: Re: [RFR]8215622: Add dump to file support for jmap histo On 2/11/2019 2:20 PM, Hohensee, Paul wrote: The CSR was just closed, so we're stuck with doing another one later. Before a changeset is pushed, its CSR can be amended (Withdraw or back to draft, then refinalize after editing.) HTH -Joe