I agree, let’s leave the dump option alone for now. Changing “and” to “or” is
just a slight grammar correctness fix.
Thanks,
Paul
From: "serguei.spit...@oracle.com" <serguei.spit...@oracle.com>
Date: Monday, February 11, 2019 at 6:33 PM
To: 臧琳 <zangl...@jd.com>, "Joseph D. Darcy" <joe.da...@oracle.com>, "Hohensee,
Paul" <hohen...@amazon.com>, David Holmes <david.hol...@oracle.com>, JC Beyler
<jcbey...@google.com>
Cc: "serviceability-dev@openjdk.java.net" <serviceability-dev@openjdk.java.net>
Subject: Re: [RFR]8215622: Add dump to file support for jmap histo
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><mailto:joe.da...@oracle.com>
Sent: Tuesday, February 12, 2019 6:22:59 AM
To: Hohensee, Paul;
serguei.spit...@oracle.com<mailto:serguei.spit...@oracle.com>; 臧琳; David
Holmes; JC Beyler
Cc:
serviceability-dev@openjdk.java.net<mailto: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
