On Wed, 31 Jul 2024 19:10:41 GMT, Henry Lin <d...@openjdk.org> wrote:
> Adds a command line option `-redact` to `jcmd`, `redact` to `jmap` and
> `-XX:+HeapDumpRedacted` enabling redacted heap dumps. When enabled, the
> output binary heap dump has zeroes written out in place of the original
> primitive values in the object fields. There is a new jtreg test
> `heapDumpRedactedTest.java` that tests that the fields are properly redacted.
Looks interesting, some suggestions:
src/hotspot/share/services/heapDumper.cpp line 999:
> 997:
> 998: // dumps the raw value of the given field
> 999: void DumperSupport::dump_field_value(AbstractDumpWriter* writer, char
> type, oop obj, int offset, bool redact) {
I think it would be cleaner to specialize this method into two:
`dump_field_value` and `dump_redacted_field_value`, and dispatch to it from
outside. Likely saves branches.
src/hotspot/share/services/heapDumper.cpp line 1386:
> 1384:
> 1385: if (redact) {
> 1386: writer->write_raw(nullptr, length_in_bytes); //nullptr to write
> zeros
Would be easier to just:
if (redact) {
writer->write_raw(nullptr, length_in_bytes); //nullptr to write zeros
writer->end_sub_record();
return;
}
...and leave the rest of the code alone.
-------------
PR Review: https://git.openjdk.org/jdk/pull/20409#pullrequestreview-2212939599
PR Review Comment: https://git.openjdk.org/jdk/pull/20409#discussion_r1700262770
PR Review Comment: https://git.openjdk.org/jdk/pull/20409#discussion_r1700258753
