This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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
David> + // 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.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]