On Wed, 4 Oct 2023 01:23:01 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> Alex Menkov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> misspell getRefererId for consistency > > src/hotspot/share/services/heapDumper.cpp line 1579: > >> 1577: >> 1578: _frames = new (mtServiceability) GrowableArray<StackFrameInfo*>(10, >> mtServiceability); >> 1579: bool stopAtVthreadEntry = _thread_type == ThreadType::MountedVirtual; > > Nit: The naming convention is not camel case, so it has to be: > `stop_at_vthread_entry`. Fixed > src/hotspot/share/services/heapDumper.cpp line 2034: > >> 2032: >> 2033: // HPROF_GC_ROOT_THREAD_OBJ records for platform and mounted virtual >> threads >> 2034: void dump_platform_threads(); > > I'd suggest to rename it to `dump_threads()` as the suffix `_platform` is > confusing as the comment says it is also dumping virtual threads. Done > src/hotspot/share/services/heapDumper.cpp line 2045: > >> 2043: >> 2044: // HPROF_TRACE and HPROF_FRAME records for platform and mounted >> virtual threads >> 2045: void dump_platform_stack_traces(); > > This name is a little confusing. I's suggest to keep `name > dump_stack_traces()`. Done ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15869#discussion_r1349412470 PR Review Comment: https://git.openjdk.org/jdk/pull/15869#discussion_r1349412550 PR Review Comment: https://git.openjdk.org/jdk/pull/15869#discussion_r1349412521
