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

[Coleen Phillimore]

On Tue, 23 Aug 2022 14:54:49 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>>>One simpler, more pragmatic way would be for the thread group to give each started thread a numeric id. Most of the cases I can think of would be happy with that, using the id inside the thread function to control behavior.
>> 
>> That might be sufficient. Looking through the test cases, they're either too complex for me to take the time to understand for this discussion, or would work OK with this kind of id. I'm thinking for N threads, each threads gets a unique id in the range `[0,N)`, are we thinking the same thing?
>> 
>>>The ability to start with the number of threads dictated at runtime, depending on runtime conditions. The ability to reuse the same TTG in different subtests with different thread numbers without template bloat.
>> 
>> There's probably no significant gain to be had in re-using the same TTG in different subtests, but I see the value of minimizing compilation times by not having several monomorphizations depending on N.
>> 
>>>Actually, the same argument goes for F. This design still requires us to use the same thread function for all threads. For cases where you want to have multiple different thread functions, you would still end up with some sort of switch inside the thread function. If F were a runtime thing, you could do something like this:
>> 
>> That's flexible, but not easier to understand. Suddenly, several `doit()` and several `join()`s are viable, it makes the semantics of TTG way more complicated. Simple tests are preferable, I'd keep `F` if it meant that we *can't* add two different functors to a TTG.
>> 
>> We *can* get rid of `F`, as so:
>> 
>> 
>> #include <functional>
>> 
>> template<typename S>
>> class BasicTestThread : public JavaTestThread {
>>   std::function<void(Thread*, S)> f;
>> };
>> 
>> 
>> Unfortunately `std::function` is a no go, but it does show an enticing way forward. It would be useful in many places of Hotspot, probably having a real build time improvement.
>> 
>> 
>> Here's my plan:
>> 
>> - Move N out from being a template parameter to a constructor argument
>> - Try to remove S and instead use a numeric id, see how the two solutions differ for these test cases. If it's nice, I'll also remove S (and therefore also state generator, for example).
>> - I'm keeping F, no changes there.
>
>> > One simpler, more pragmatic way would be for the thread group to give each started thread a numeric id. Most of the cases I can think of would be happy with that, using the id inside the thread function to control behavior.
>> 
>> That might be sufficient. Looking through the test cases, they're either too complex for me to take the time to understand for this discussion, or would work OK with this kind of id. I'm thinking for N threads, each threads gets a unique id in the range `[0,N)`, are we thinking the same thing?
> 
> Yes. At least the tests I wrote, I usually had like one thread that would do A, the rest B, or half/half would do A or B. That can be just controlled via the running id. Beside its nice for debugging.
> 
>> 
>> > The ability to start with the number of threads dictated at runtime, depending on runtime conditions. The ability to reuse the same TTG in different subtests with different thread numbers without template bloat.
>> 
>> There's probably no significant gain to be had in re-using the same TTG in different subtests, but I see the value of minimizing compilation times by not having several monomorphizations depending on N.
> 
> Not sure what you meant with the first half of the sentence. But one example for controlling num threads at runtime, I think now that we add more and more functionality to gtest, we may need a way to control the test width. Reduce the number of concurrent threads. There are folks running these tests as part of their build on tiny make machines. Of course, canonical test results would still require a certain number of concurrent tests.
> 
>> 
>> > Actually, the same argument goes for F. This design still requires us to use the same thread function for all threads. For cases where you want to have multiple different thread functions, you would still end up with some sort of switch inside the thread function. If F were a runtime thing, you could do something like this:
>> 
>> That's flexible, but not easier to understand. Suddenly, several `doit()` and several `join()`s are viable, it makes the semantics of TTG way more complicated. Simple tests are preferable, I'd keep `F` if it meant that we _can't_ add two different functors to a TTG.
> 
> Ok, I agree.
> 
>> 
>> We _can_ get rid of `F`, as so:
>> 
>> ```
>> #include <functional>
>> 
>> template<typename S>
>> class BasicTestThread : public JavaTestThread {
>>   std::function<void(Thread*, S)> f;
>> };
>> ```
>> 
>> Unfortunately `std::function` is a no go, but it does show an enticing way forward. It would be useful in many places of Hotspot, probably having a real build time improvement.
>> 
>> Here's my plan:
>> 
>> * Move N out from being a template parameter to a constructor argument
>> * Try to remove S and instead use a numeric id, see how the two solutions differ for these test cases. If it's nice, I'll also remove S (and therefore also state generator, for example).
>> * I'm keeping F, no changes there.
> 
> Sounds good. Thanks for considering this.
> 
> Cheers, Thomas

I have to say that as someone who just learned what declspec was yesterday, a little bit of documentation would help to know what the template parameters should be, but otherwise, this is much better than copying and pasting the thread group code with semaphores.  Writing the lambda to do the task, and the number of threads to do that is very nice.  Maybe the example changes should give a name to the second lambda for StateFunc.
I agree builds are getting really slow though, but I don't know if this makes it worse.  I'd rather not copy this code.

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

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