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