[Patch] Java: Add heap dump and analyze support.
Boehm, Hans
hans.boehm@hp.com
Tue Jan 30 22:21:00 GMT 2007
Sounds fine to me. I just didn't want to discourage patches to the GC
in this area.
Hans
> -----Original Message-----
> From: David Daney [mailto:ddaney@avtrex.com]
> Sent: Tuesday, January 30, 2007 1:57 PM
> To: Boehm, Hans
> Cc: tromey@redhat.com; Johannes P. Schmidt; Java Patch List
> Subject: Re: [Patch] Java: Add heap dump and analyze support.
>
> Boehm, Hans wrote:
> > I think the only partial excuse for the ugly way this is handled in
> > the GC is that it tries to avoid stdio, in order to avoid
> accidentally
> > calling malloc. This probably doesn't matter for gcj, but it's
> > important if malloc is actually an alias for GC_malloc, which it
> > sometimes is.
> >
> > Nonetheless, this should probably be parameterized with respect to
> > file descriptors (probably not FILE *) on Posix systems, and the
> > approximate equivalent on other systems.
> >
> > GC7 handles this slightly better, but not a lot.
> >
>
> I would propose that we commit the patch much as it is.
>
> If the GC could add an entry point to allow the information
> to be printed to an arbitrary FILE* or file descriptor, then
> we would adjust the heap dumper when that is imported into GCC.
>
> Does that sound reasonable?
>
> David Daney
>
> > Hans
> >
> >> -----Original Message-----
> >> From: java-patches-owner@gcc.gnu.org
> >> [mailto:java-patches-owner@gcc.gnu.org] On Behalf Of David Daney
> >> Sent: Monday, January 29, 2007 3:58 PM
> >> To: tromey@redhat.com
> >> Cc: Java Patch List; gcc-patches@gcc.gnu.org; Johannes P. Schmidt
> >> Subject: Re: [Patch] Java: Add heap dump and analyze support.
> >>
> >> New version of the patch:
> >>
> >> Tom Tromey wrote:
> >>>>>>>> "David" == David Daney <ddaney@avtrex.com> writes:
> >>>>>>>>
> >>> 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.
> >>>
> >> As I said before: I look at this quite a bit. The GC has
> code that
> >> prints the needed information to stdout.
> >> Unfortunately we need it in a file. I was thinking about
> modifying
> >> the GC so that you could supply a file descriptor, but
> that code has
> >> #ifdefs for every kind of OS known to man, and I was
> afraid I would
> >> break some obscure untestable configuration.
> >>> 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.
> >>>
> >>>
> >> Fixed.
> >>> 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 // return -1 if
> none found
> >>> David> + long getAddress(SymbolLookup lookup, String
> >> symbol) throws
> >>> David> + IOException {
> >>> David> + // for (Iterator
> >> it=map.entrySet().iterator(); it.hasNext(); )
> >>> David> + // {
> >>>
> >>> Why is this commented out?
> >>>
> >> Unused code. It has been removed.
> >>> David> + if (s.charAt(0) == '#')
> >>> David> + continue;
> >>> David> + if (s == null)
> >>> David> + break;
> >>>
> >>> These tests are in the wrong order.
> >>>
> >> Fixed.
> >>> David> + static void usage()
> >>> David> + {
> >>>
> >>> Someday I suppose we should convert the tools in gcj to use the
> >>> classpath getopt code.
> >>>
> >> Done.
> >>> David> + if
> >> (name.compareTo("gnu.gcj.runtime.MethodRef")
> >>> David> + == 0)
> >>>
> >>> What is this?
> >>>
> >> Bogus code left over from 3.4.3 version. It has been removed.
> >>> David> + public static String KindToName(int kind)
> >>>
> >>> Non-java name... method names should start with a lower
> case letter.
> >>>
> >> Fixed.
> >>> 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.
> >>>
> >> My misunderstanding of namespaces. Changed to anonymous
> namespace as
> >> suggested by Joel Dice and Chris Lattner.
> >>
> >> Tested on x86_64-pc-linux-gnu (FC6) with no regressions.
> >> Currently testing on i686-pc-linux-gnu.
> >>
> >> OK to commit if it passes?
> >> 2007-01-29 David Daney <ddaney@avtrex.com>
> >> gcc/java:
> >> * Make-lang.in (JAVA_MANFILES): Added doc/gc-analyze.1.
> >> (java.maintainer-clean):Added gc-analyze.1.
> >> (.INTERMEDIATE): Added gc-analyze.pod.
> >> (gc-analyze.pod): New rule.
> >> (java.install-man): Install gc-analyze.1
> >> * gcj.texi: Added new section for the gc-analyze program.
> >>
> >> libjava:
> >> 2007-01-29 Johannes Schmidt <jschmidt@avtrex.com>
> >> David Daney <ddaney@avtrex.com>
> >>
> >> * configure.ac: Added check for /proc/self/maps.
> >> * Makefile.am (bin_PROGRAMS): Added gc-analyze.
> >> (AM_GCJFLAGS): Set classpath to
> $(srcdir)/classpath/tools/classes.
> >> (gcjh.stamp): Same.
> >> (gc_analyze_SOURCES): New.
> >> (gc_analyze_LDFLAGS): New.
> >> (gc_analyze_LINK): New.
> >> (gc_analyze_LDADD): New.
> >> (gc_analyze_DEPENDENCIES): New.
> >> (nat_source_files): Added gnu/gcj/util/natGCInfo.cc.
> >> * Makefile.in: Regenerated.
> >> * configure: Regenerated.
> >> * include/config.h.in: Regenerated.
> >> * sources.am: Regenerated.
> >> * scripts/makemake.tcl: Don't include gc-analyze classes in
> >> libgcj.
> >> * gnu/classpath/tools/getopt/Parser.h: New.
> >> * gnu/classpath/tools/getopt/FileArgumentCallback.h: New.
> >> * gnu/classpath/tools/getopt/Parser$1.h: New.
> >> * gnu/classpath/tools/getopt/Parser$2.h: New.
> >> * gnu/classpath/tools/getopt/Parser$3.h: New.
> >> * gnu/classpath/tools/getopt/OptionGroup.h: New.
> >> * gnu/classpath/tools/getopt/OptionException.h: New.
> >> * gnu/classpath/tools/getopt/Messages.h: New.
> >> * gnu/classpath/tools/getopt/Option.h: New.
> >> *
> >> gnu/gcj/tools/gc_analyze/MemoryAnalyze$SubstringComparator.h: New.
> >> * gnu/gcj/tools/gc_analyze/SymbolLookup.java: New.
> >> * gnu/gcj/tools/gc_analyze/BytePtr.h: New.
> >> * gnu/gcj/tools/gc_analyze/ItemList.h: New.
> >> * gnu/gcj/tools/gc_analyze/ToolPrefix.h: New.
> >> * gnu/gcj/tools/gc_analyze/MemoryAnalyze.h: New.
> >> * gnu/gcj/tools/gc_analyze/MemoryAnalyze$1.h: New
> >> * gnu/gcj/tools/gc_analyze/MemoryAnalyze$2.h: New
> >> * gnu/gcj/tools/gc_analyze/MemoryAnalyze$3.h: New
> >> * gnu/gcj/tools/gc_analyze/BlockMap$SizeKind.h: New.
> >> * gnu/gcj/tools/gc_analyze/ObjectMap.java: New.
> >> * gnu/gcj/tools/gc_analyze/SymbolLookup.h: New.
> >> * gnu/gcj/tools/gc_analyze/MemoryMap.java: New.
> >> * gnu/gcj/tools/gc_analyze/MemoryAnalyze$1$Info.h: New.
> >> * gnu/gcj/tools/gc_analyze/ObjectMap.h: New.
> >> * gnu/gcj/tools/gc_analyze/MemoryMap.h: New.
> >> * gnu/gcj/tools/gc_analyze/SymbolTable.java: New.
> >> * gnu/gcj/tools/gc_analyze/SymbolTable.h: New.
> >> * gnu/gcj/tools/gc_analyze/ObjectMap$ObjectItem.h: New.
> >> * gnu/gcj/tools/gc_analyze/MemoryMap$RangeComparator.h: New.
> >> * gnu/gcj/tools/gc_analyze/BlockMap$PtrMarks.h: New.
> >> * gnu/gcj/tools/gc_analyze/BlockMap.java: New.
> >> * gnu/gcj/tools/gc_analyze/BytePtr.java: New.
> >> * gnu/gcj/tools/gc_analyze/ItemList.java: New.
> >> * gnu/gcj/tools/gc_analyze/ToolPrefix.java: New.
> >> * gnu/gcj/tools/gc_analyze/MemoryAnalyze.java: New.
> >> * gnu/gcj/tools/gc_analyze/MemoryMap$Range.h: New.
> >> * gnu/gcj/tools/gc_analyze/BlockMap.h: New.
> >> * gnu/gcj/util/GCInfo.java: New.
> >> * gnu/gcj/util/GCInfo.h: New.
> >> * gnu/gcj/util/natGCInfo.cc: New.
> >> * gnu/gcj/util/UtilPermission.java: New.
> >> * gnu/gcj/util/UtilPermission.h: New.
> >> * classpath/lib/Makefile.am (gcj_tools_classpath): New
> variable.
> >> (compile_classpath): Add $(gcj_tools_classpath) to classpath.
> >> * classpath/lib/Makefile.in: Regenerated.
> >> *
> classpath/lib/gnu/gcj/tools/gc_analyze/SymbolTable.class: New.
> >> *
> >> classpath/lib/gnu/gcj/tools/gc_analyze/ObjectMap$ObjectItem.class:
> >> New.
> >> *
> >> classpath/lib/gnu/gcj/tools/gc_analyze/MemoryMap$RangeComparat
> >> or.class: New.
> >> *
> >>
> classpath/lib/gnu/gcj/tools/gc_analyze/BlockMap$PtrMarks.class: New.
> >> *
> >> classpath/lib/gnu/gcj/tools/gc_analyze/MemoryMap$Range.class: New.
> >> * classpath/lib/gnu/gcj/tools/gc_analyze/BlockMap.class: New.
> >> * classpath/lib/gnu/gcj/tools/gc_analyze/BytePtr.class: New.
> >> *
> >> classpath/lib/gnu/gcj/tools/gc_analyze/MemoryAnalyze$Substring
> >> Comparator.class:
> >> New.
> >> * classpath/lib/gnu/gcj/tools/gc_analyze/ItemList.class: New.
> >> * classpath/lib/gnu/gcj/tools/gc_analyze/ToolPrefix.class: New.
> >> *
> classpath/lib/gnu/gcj/tools/gc_analyze/MemoryAnalyze.class: New.
> >> *
> >>
> classpath/lib/gnu/gcj/tools/gc_analyze/BlockMap$SizeKind.class: New.
> >> *
> classpath/lib/gnu/gcj/tools/gc_analyze/SymbolLookup.class: New.
> >> *
> >> classpath/lib/gnu/gcj/tools/gc_analyze/MemoryAnalyze$1$Info.class:
> >> New.
> >> * classpath/lib/gnu/gcj/tools/gc_analyze/ObjectMap.class: New.
> >> * classpath/lib/gnu/gcj/tools/gc_analyze/MemoryMap.class: New.
> >> * classpath/lib/gnu/gcj/util/GCInfo.class: New.
> >> * classpath/lib/gnu/gcj/util/UtilPermission.class: New.
> >>
> >> As before machine generated files are omitted from the diff.
> >>
> >>
>
>
More information about the Java-patches
mailing list