This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch] Java: Add heap dump and analyze support.
- From: Tom Tromey <tromey at redhat dot com>
- To: David Daney <ddaney at avtrex dot com>
- Cc: Java Patch List <java-patches at gcc dot gnu dot org>, gcc-patches at gcc dot gnu dot org, "Johannes P. Schmidt" <jschmidt at avtrex dot com>
- Date: 23 Jan 2007 13:09:06 -0700
- Subject: Re: [Patch] Java: Add heap dump and analyze support.
- References: <45AEB57B.5090807@avtrex.com>
- Reply-to: tromey at redhat dot com
>>>>> "David" == David Daney <ddaney@avtrex.com> writes:
Sorry for the delay on this.
David> This patch adds the ability to dump the contents of the runtime heap
David> to a file from a java program.
Nice.
I read through this. I don't see many issues -- a few formatting
things (on irc you said these are fixed).
I'm a little concerned about the ad hoc approach to demangling, but
not enough to want to change it.
I'm also worried about the GC private bits copied into the native
code. My main concern is that if the GC changes, we won't notice.
My final high-level worry is about security. Right now it is possible
for any program to call the code that dumps the memory map. I suppose
we can just sweep this under the blanket we're putting 'gnu.classpath'
stuff under.
Please comment on these.
I have a few more minor comments.
David> +@item -p @var{tool-prefix}
David> +Prefix added to the names of the @command{nm} and @command{readelf} commands.
I'd prefer to compile in the target, but I suppose this tool isn't
built in non-native builds, so that wouldn't help. Darn.
David> + // search all symbol tables
David> + // return -1 if none found
David> + long getAddress(SymbolLookup lookup, String symbol) throws IOException
David> + {
David> + // for (Iterator it=map.entrySet().iterator(); it.hasNext(); )
David> + // {
Why is this commented out?
David> + if (s.charAt(0) == '#')
David> + continue;
David> + if (s == null)
David> + break;
These tests are in the wrong order.
David> + static void usage()
David> + {
Someday I suppose we should convert the tools in gcj to use the
classpath getopt code.
David> + if (name.compareTo("gnu.gcj.runtime.MethodRef") == 0)
What is this?
David> + public static String KindToName(int kind)
Non-java name... method names should start with a lower case letter.
David> +namespace gnu
David> +{
David> + namespace gcj
David> + {
David> + namespace util
David> + {
David> + class GC_enumerator
This appears to be local to a single .cc file. Do we need to put it
into a namespace like this? I've always thought of these namespaces
as reserved for 'extern "Java"' code.
Tom