RFR: 8292679: Simplify thread creation in gtest and port 2 tests to new way

[David Holmes]

On Mon, 22 Aug 2022 17:06:51 GMT, Johan Sjölén <duke at openjdk.org> wrote:

>> test/hotspot/gtest/threadHelper.inline.hpp line 148:
>> 
>>> 146: };
>>> 147: 
>>> 148: // A TestThreadGroup tracks multiple threads running the same function.
>> 
>> So we have F, which is kind of clear, S, which is a state?, and then also a state generator? Which apparently needs to define operator () and return a state? Could you please document what the template parameters are and what is expected of them?
>> 
>> Does N really need to be a template parameter? You lose flexibility, and it is less readable too.
>
>>So we have F, which is kind of clear, S, which is a state?, and then also a state generator? Which apparently needs to define operator () and return a state?
> 
> Let me explain my reasoning, call me out if something seems odd please!
> 
> A `BasicTestThread` has a F, the code which runs, and S, the local state which is only available for F. A ´TestThreadGroup` let's you create many `BasicTestThread` for the same F, but with varying S, with the help of a state generator.
> 
> Subclasses (the "old" way) are good at having their own local state, whilst lambdas are good at sharing state (through their access to the lexical environment they're created in). The whole S and state generator business is kind of a crutch to help lambdas out to be more like subclasses. Maybe there's a bit of YAGNI here, and really it's fine if `TestThreadGroup` also gets an S like `BasicTestThread` does?
> 
>>Could you please document what the template parameters are and what is expected of them?
> 
> Absolutely. I'll also lengthen their name, to make them more clear-
> 
>>Does N really need to be a template parameter? You lose flexibility, and it is less readable too.
> 
> What flexibility are we losing? We're not going to be dynamically reallocate the amount of threads in a `TestThreadGroup`, or is this the exact use case you're imagining in the future?
> 
> I don't think it's necessarily less readable. It's fairly standard in C++ STL to use template arguments for sizes (see `std::array`). I know we typically use raw pointers and length variables in Hotspot, so it's certainly unidiomatic from those eyes. To me, it's more readable, because it's a hint to me that "aha, this size won't change during execution".

The experience of developers with templates and STL will vary widely - just something to bear in mind when modernising the codebase. :) I found it harder to "spot the threads" in the new style.

-------------

PR: https://git.openjdk.org/jdk/pull/9962