EVALUATION

http://hg.openjdk.java.net/hsx/hsx16/master/rev/b0651468aff7
                                     

EVALUATION

http://hg.openjdk.java.net/hsx/hsx16/baseline/rev/b0651468aff7
                                     

EVALUATION

http://hg.openjdk.java.net/jdk7/hotspot-gc/hotspot/rev/72a6752ac432
                                     

EVALUATION

Reopening based upon discussion with colleagues in tools group.
This will be fixed to restore the original expected behaviour. So that
the temporary change in behaviour is not externally visible in a public
JDK, I'm opening a subCR to fix this in hs16/6u18 as well.
                                     

SUGGESTED FIX

diff -r b0b36f0de97e src/share/vm/services/heapDumper.cpp
--- a/src/share/vm/services/heapDumper.cpp      Tue Oct 20 11:28:25 2009 -0700
+++ b/src/share/vm/services/heapDumper.cpp      Tue Oct 27 14:39:35 2009 -0700
@@ -1913,8 +1913,9 @@ void HeapDumper::dump_heap() {
     if (use_default_filename) {
       char fn[32];
       sprintf(fn, "java_pid%d", os::current_process_id());
-      assert(strlen(base_path) + strlen(fn) < sizeof(base_path), "HeapDumpPath too long");
+      assert(strlen(base_path) + strlen(fn) + strlen(".hprof") < sizeof(base_path), "HeapDumpPath too long");
       strcat(base_path, fn);
+      strcat(base_path, ".hprof");
     }
     assert(strlen(base_path) < sizeof(my_path), "Buffer too small");
     strcpy(my_path, base_path);
@@ -1927,8 +1928,6 @@ void HeapDumper::dump_heap() {
     strcat(my_path, fn);
   }
   dump_file_seq++;   // increment seq number for next time we dump
-  assert(strlen(".hprof") + strlen(my_path) < sizeof(my_path), "HeapDumpPath too long");
-  strcat(my_path, ".hprof");
 
   HeapDumper dumper(false /* no GC before heap dump */,
                     true  /* send to tty */);
                                     

EVALUATION

This was a consequence of the changes in 6797870, and as the
submitter indicated the behaviour does indeed change as of this
CR. However, it appears as though the current behaviour of
always appending a ".hprof" extension to the path specified
might be the more desirable behaviour given the extension of
the HeapDumpPath variable for use in repeated heap dumps
that also append a sequence number extension.

As such I propose that we leave this change in behaviour intact, and we
instead adjust the documentation of the flag so it is clear that
a .hprof extension is always appended to the path specified in
HeapDumpPath.

I'll consult with the tools/serviceability team to see if they
agree with a "will not fix" for this. NOTE: while the change in behaviour
is easy to reverse, i think it will unnecessarily cause a kink in
the specification of the path in the case where you need repeated heap dumps...
                                     

EVALUATION

As the submitter surmised, this is likely a result of the changes in 6797870.
I'll take care of it so that the old behaviour is restored.