Hi Paul,
I think it is not necessary to have a loop for argument processing, since
there is not so much arguments to handle.
And I made a new webrev
http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/
Hi All,
May I also ask your help for reviewing this webrev?
webrev http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8215622
Thanks!
Lin
________________________________
发件人: serviceability-dev <serviceability-dev-boun...@openjdk.java.net> 代表 臧琳
<zangl...@jd.com>
发送时间: 2019年1月3日 10:21
收件人: Hohensee, Paul; serviceability-dev@openjdk.java.net
主题: [发件地址伪造,恶意邮件,勿点] RE: [RFR]8215622: Add dump to file support for jmap histo
Dear Paul,
Thanks a lot for your review! I am trying to remend the patch
One problem is that in future, I will add more options for histo, such
as “parallel”, “incremental”.
so do you thinking option processing with loop in heap_inspection() in
attachListener.cpp would
be more reasonable than the “if else” ?
Thanks
BRs,
Lin
From: Hohensee, Paul <hohen...@amazon.com>
Sent: Saturday, December 29, 2018 3:54 AM
To: 臧琳 <zangl...@jd.com>; serviceability-dev@openjdk.java.net
Subject: Re: [RFR]8215622: Add dump to file support for jmap histo
Update the copyright dates to 2019 please, since this won’t get pushed until
then.
In JMap.java, I’d emulate dump() and use
String filename = null;
String subopts[] = options.split(",");
for (String subopt : subopts){
if (subopt.isEmpty() || subopt.equals("all")) {
// pass
} else if (subopt.equals("live")) {
liveopt = "-live";
} else if (subopt.startsWith("file=")) {
// file=<file> - check that <file> is specified
if (subopt.length() > 5) {
filename = subopt.substring(5);
}
} else {
usage(1);
}
}
// get the canonical path - important to avoid just passing
// a "heap.bin" and having the dump created in the target VM
// working directory rather than the directory where jmap
// is executed.
filename = new File(filename).getCanonicalPath();
// inspectHeap is not the same as jcmd GC.class_histogram
executeCommandForPid(pid, "inspectheap", filename, liveopt);
I.e., use an enhanced for loop to scan the array, and duplicate dump()’s
executeCommandForPid() argument order, as well as dump()’s “file=<>” check (the
code that starts with “if (subopt.startsWith”) and canonicalization. Actually,
better to factor the latter out into its own method and use it from both
histo() and dump().
The argument checking code in heap_inspection() in attachListener.cpp can be
simplified along the lines of dump_heap(). I.e., you don’t need to loop over
the argument list. To match up with dump_heap()’s info messages, the info
message string at the end should be “Heap inspection file created: %s”.
Thanks,
Paul
From: serviceability-dev
<serviceability-dev-boun...@openjdk.java.net<mailto:serviceability-dev-boun...@openjdk.java.net>>
on behalf of 臧琳 <zangl...@jd.com<mailto:zangl...@jd.com>>
Date: Thursday, December 20, 2018 at 11:03 PM
To:
"serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>"
<serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>>
Subject: [RFR]8215622: Add dump to file support for jmap histo
Hi All,
May I ask your help to review this patch for enhance jmap �Chisto.
It add the “file=<path>” arguments that allow jmap �Chisto outputs data to file
directly.
This patch is also part of the enhancement described in
https://bugs.openjdk.java.net/browse/JDK-8214535.
Webrev: http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8215622
Thanks.
BRs,
Lin
