RFR: 8312132: Add tracking of multiple address spaces in NMT [v105]
Johan Sjölen
jsjolen at openjdk.org
Thu May 23 16:25:15 UTC 2024
On Thu, 23 May 2024 04:48:18 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> test/hotspot/gtest/nmt/test_vmatree.cpp line 71:
>>
>>> 69: NativeCallStackStorage::StackIndex si1 = ncs.push(stack1);
>>> 70: NativeCallStackStorage::StackIndex si2 = ncs.push(stack2);
>>> 71:
>>
>> Please make these auto functions normal conventional functions. I really dislike that style.
>>
>> It has real disadvantages. For example, no IDE I know can resolve a call graph across them, or even into them. E.g., in CDT, one of the most capable C++ IDEs, the call graph for VMATreeTest::in_type_of gives me .... nothing. I need to do a dumb full text search to find the call sites. And if the term to search for is very generic, I am out of luck. I know several developers that rely heavily on call graph search in CDT, I certainly do.
>>
>> New techniques should serve a purpose. Lambdas as replacement for our old Closures makes sense, since they bring benefit (avoiding runtime polymorphy). But here I don't see the point.
>
> Also, please split all of these scopes up into individual TESTs. A finer granularity allows you to reproduce individual TESTS without having to sit through 20+ irrelevant ones. It also makes for cleaner code.
I didn't know about the call graph issue, thanks for explaining. Splitting these out into explicit tests.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1611986725
More information about the hotspot-dev
mailing list