Hi Thomas, thanks for the feedback.
> In DumpWriter, _current_entry_left and _entry_ended seem only to be needed for > asserting. Please enclose their definition in DEBUG_ONLY, and initialize them > in the ctor. Good catch. I made them debug only. > (not your patch): since DumperSupport::dump_class_and_array_classes(Klass*) > should assert that Klass* is an InstanceKlass; or, even better, use > InstanceKlass* > as parameter. A former version of the patch had a lot of the Klass* types replaced by InstanceKlass* where appropriate. I removed those changes ultimately because they had not much to with making the heap dump streamable. But a later patch could change this. > DumpWriter::start_dump_entry(): It took me a while to understand how the > segment size is updated if the entry is huge, since by the time we finish the > entry the segment header will already be flushed out. The answer is, I think, > that this is not needed since we only write one record so the initial size we > wrote > into the segment header is still valid. Correct. > Proposed comment change: > > -// Will be fixed up later if we can add more entries. > +// Seed segment size with size of its first record. Should we add more > records later, > we will update the segment size (see >finish_dump_segment()) I’ve changed the comment to make it more clear. Best regards, Ralf
