Skip to content

Commit

Permalink
8267666: Add option to jcmd GC.heap_dump to use existing file
Browse files Browse the repository at this point in the history 
Reviewed-by: rschmelter, clanger
  • Loading branch information
@AntonKozlov
AntonKozlov committed Jul 12, 2021
1 parent 8973867 commit 7cbb67a
Show file tree
Hide file tree
Showing 11 changed files with 27 additions and 23 deletions.
4 changes: 1 addition & 3 deletions src/hotspot/os/aix/os_aix.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2660,9 +2660,7 @@ int os::open(const char *path, int oflag, int mode) {
// create binary file, rewriting existing file if required
int os::create_binary_file(const char* path, bool rewrite_existing) {
int oflags = O_WRONLY | O_CREAT;
if (!rewrite_existing) {
oflags |= O_EXCL;
}
oflags |= rewrite_existing ? O_TRUNC : O_EXCL;
return ::open64(path, oflags, S_IREAD | S_IWRITE);
}

Expand Down
4 changes: 1 addition & 3 deletions src/hotspot/os/bsd/os_bsd.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2350,9 +2350,7 @@ int os::open(const char *path, int oflag, int mode) {
// create binary file, rewriting existing file if required
int os::create_binary_file(const char* path, bool rewrite_existing) {
int oflags = O_WRONLY | O_CREAT;
if (!rewrite_existing) {
oflags |= O_EXCL;
}
oflags |= rewrite_existing ? O_TRUNC : O_EXCL;
return ::open(path, oflags, S_IREAD | S_IWRITE);
}

Expand Down
4 changes: 1 addition & 3 deletions src/hotspot/os/linux/os_linux.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4963,9 +4963,7 @@ int os::open(const char *path, int oflag, int mode) {
// create binary file, rewriting existing file if required
int os::create_binary_file(const char* path, bool rewrite_existing) {
int oflags = O_WRONLY | O_CREAT;
if (!rewrite_existing) {
oflags |= O_EXCL;
}
oflags |= rewrite_existing ? O_TRUNC : O_EXCL;
return ::open64(path, oflags, S_IREAD | S_IWRITE);
}

Expand Down
4 changes: 1 addition & 3 deletions src/hotspot/os/windows/os_windows.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4801,9 +4801,7 @@ bool os::dir_is_empty(const char* path) {
// create binary file, rewriting existing file if required
int os::create_binary_file(const char* path, bool rewrite_existing) {
int oflags = _O_CREAT | _O_WRONLY | _O_BINARY;
if (!rewrite_existing) {
oflags |= _O_EXCL;
}
oflags |= rewrite_existing ? _O_TRUNC : _O_EXCL;
return ::open(path, oflags, _S_IREAD | _S_IWRITE);
}

Expand Down
7 changes: 5 additions & 2 deletions src/hotspot/share/services/diagnosticCommand.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,10 +468,13 @@ HeapDumpDCmd::HeapDumpDCmd(outputStream* output, bool heap) :
"BOOLEAN", false, "false"),
_gzip("-gz", "If specified, the heap dump is written in gzipped format "
"using the given compression level. 1 (recommended) is the fastest, "
"9 the strongest compression.", "INT", false, "1") {
"9 the strongest compression.", "INT", false, "1"),
_overwrite("-overwrite", "If specified, the dump file will be overwritten if it exists",
"BOOLEAN", false, "false") {
_dcmdparser.add_dcmd_option(&_all);
_dcmdparser.add_dcmd_argument(&_filename);
_dcmdparser.add_dcmd_option(&_gzip);
_dcmdparser.add_dcmd_option(&_overwrite);
}

void HeapDumpDCmd::execute(DCmdSource source, TRAPS) {
Expand All @@ -490,7 +493,7 @@ void HeapDumpDCmd::execute(DCmdSource source, TRAPS) {
// This helps reduces the amount of unreachable objects in the dump
// and makes it easier to browse.
HeapDumper dumper(!_all.value() /* request GC if _all is false*/);
dumper.dump(_filename.value(), output(), (int) level);
dumper.dump(_filename.value(), output(), (int) level, _overwrite.value());
}

ClassHistogramDCmd::ClassHistogramDCmd(outputStream* output, bool heap) :
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/services/diagnosticCommand.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ class HeapDumpDCmd : public DCmdWithParser {
DCmdArgument<char*> _filename;
DCmdArgument<bool> _all;
DCmdArgument<jlong> _gzip;
DCmdArgument<bool> _overwrite;
public:
HeapDumpDCmd(outputStream* output, bool heap);
static const char* name() {
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/services/heapDumper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1905,7 +1905,7 @@ void VM_HeapDumper::dump_stack_traces() {
}

// dump the heap to given path.
int HeapDumper::dump(const char* path, outputStream* out, int compression) {
int HeapDumper::dump(const char* path, outputStream* out, int compression, bool overwrite) {
assert(path != NULL && strlen(path) > 0, "path missing");

// print message in interactive case
Expand All @@ -1928,7 +1928,7 @@ int HeapDumper::dump(const char* path, outputStream* out, int compression) {
}
}

DumpWriter writer(new (std::nothrow) FileWriter(path), compressor);
DumpWriter writer(new (std::nothrow) FileWriter(path, overwrite), compressor);

if (writer.error() != NULL) {
set_error(writer.error());
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/services/heapDumper.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class HeapDumper : public StackObj {
// dumps the heap to the specified file, returns 0 if success.
// additional info is written to out if not NULL.
// compression >= 0 creates a gzipped file with the given compression level.
int dump(const char* path, outputStream* out = NULL, int compression = -1);
int dump(const char* path, outputStream* out = NULL, int compression = -1, bool overwrite = false);

// returns error message (resource allocated), or NULL if no error
char* error_as_C_string() const;
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/services/heapDumperCompression.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
char const* FileWriter::open_writer() {
assert(_fd < 0, "Must not already be open");

_fd = os::create_binary_file(_path, false); // don't replace existing file
_fd = os::create_binary_file(_path, _overwrite);

if (_fd < 0) {
return os::strerror(errno);
Expand Down
3 changes: 2 additions & 1 deletion src/hotspot/share/services/heapDumperCompression.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,11 @@ class AbstractWriter : public CHeapObj<mtInternal> {
class FileWriter : public AbstractWriter {
private:
char const* _path;
bool _overwrite;
int _fd;

public:
FileWriter(char const* path) : _path(path), _fd(-1) { }
FileWriter(char const* path, bool overwrite) : _path(path), _overwrite(overwrite), _fd(-1) { }

~FileWriter();

Expand Down
15 changes: 11 additions & 4 deletions test/hotspot/jtreg/serviceability/dcmd/gc/HeapDumpTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,15 @@
public class HeapDumpTest {
protected String heapDumpArgs = "";

public void run(CommandExecutor executor) throws IOException {
public void run(CommandExecutor executor, boolean overwrite) throws IOException {
File dump = new File("jcmd.gc.heap_dump." + System.currentTimeMillis() + ".hprof");
if (dump.exists()) {
if (!overwrite && dump.exists()) {
dump.delete();
} else if (overwrite) {
dump.createNewFile();
}

String cmd = "GC.heap_dump " + heapDumpArgs + " " + dump.getAbsolutePath();
String cmd = "GC.heap_dump " + (overwrite ? "-overwrite " : "") + heapDumpArgs + " " + dump.getAbsolutePath();
executor.execute(cmd);

verifyHeapDump(dump);
Expand Down Expand Up @@ -85,7 +87,12 @@ private void verifyHeapDump(File dump) {
/* GC.heap_dump is not available over JMX, running jcmd pid executor instead */
@Test
public void pid() throws IOException {
run(new PidJcmdExecutor());
run(new PidJcmdExecutor(), false);
}

@Test
public void pidRewrite() throws IOException {
run(new PidJcmdExecutor(), true);
}
}

5 comments on commit 7cbb67a

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

@RealCLanger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk17u

@openjdk
Copy link

@openjdk openjdk bot commented on 7cbb67a Jul 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RealCLanger the backport was successfully created on the branch RealCLanger-backport-7cbb67a3 in my personal fork of openjdk/jdk17u. To create a pull request with this backport targeting openjdk/jdk17u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

this pull request contains a backport of commit 7cbb67a3 from the openjdk/jdk repository.

The commit being backported was authored by Anton Kozlov on 12 Jul 2021 and was reviewed by Ralf Schmelter and Christoph Langer.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u:

$ git fetch https://github.com/openjdk-bots/jdk17u RealCLanger-backport-7cbb67a3:RealCLanger-backport-7cbb67a3
$ git checkout RealCLanger-backport-7cbb67a3
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u RealCLanger-backport-7cbb67a3

@RealCLanger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk11u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 7cbb67a Jul 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RealCLanger could not automatically backport 7cbb67a3 to openjdk/jdk11u-dev due to conflicts in the following files:

  • src/hotspot/share/services/diagnosticCommand.cpp
  • src/hotspot/share/services/diagnosticCommand.hpp
  • src/hotspot/share/services/heapDumper.cpp
  • src/hotspot/share/services/heapDumper.hpp
  • src/hotspot/share/services/heapDumperCompression.cpp
  • src/hotspot/share/services/heapDumperCompression.hpp

To manually resolve these conflicts run the following commands in your personal fork of openjdk/jdk11u-dev:

$ git checkout -b RealCLanger-backport-7cbb67a3
$ git fetch --no-tags https://git.openjdk.java.net/jdk 7cbb67a3f8adc83a5b51c092a66480d7b22a6bea
$ git cherry-pick --no-commit 7cbb67a3f8adc83a5b51c092a66480d7b22a6bea
$ # Resolve conflicts
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 7cbb67a3f8adc83a5b51c092a66480d7b22a6bea'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk11u-dev with the title Backport 7cbb67a3f8adc83a5b51c092a66480d7b22a6bea.

Please sign in to comment.