RFR(s): 8179327: gtestLauncher should run tests on a separate thread (optionally)
Mikael Gerdin
mikael.gerdin at oracle.com
Mon Jul 17 08:56:44 UTC 2017
Hi Thomas,
On 2017-07-13 08:18, Thomas Stüfe wrote:
> Ping..
>
> May I please get a second review and a sponsor?
I would prefer if you made a
const static bool DEFAULT_SPAWN_IN_NEW_THREAD which is initialized with
the proper platform value. The current 5-line return statement in
get_spawn_new_main_thread_arg looks a bit weird.
If you move the wrapping of argc/argv up to runUnitTests and pass the
args_t instead you can make run_in_new_thread completely platform
specific instead of having #ifdef _WIN32 in the middle, I think this is
the preferred style in the code base overall.
In the pthread join error path you have
"/rsyn" at the end of the line, is that intentional?
Thanks
/Mikael
>
> Thanks, Thomas
>
> On Tue, Jul 4, 2017 at 11:12 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
> wrote:
>
>>
>>
>> On Tue, Jul 4, 2017 at 10:22 AM, Robbin Ehn <robbin.ehn at oracle.com> wrote:
>>
>>> Looks reasonable, thanks!
>>>
>>>
>> Thank you!
>>
>>
>>> /Robbin
>>>
>>> If you don't mind, please don't wrap lines, links do not work well with
>>> line breaks in them :)
>>>
>>>
>> Oops, sorry!
>>
>>
>>> On 07/02/2017 09:23 AM, Thomas Stüfe wrote:
>>>
>>>> Dear all,
>>>>
>>>> please review the following change:
>>>>
>>>> Bug: JDK-8179327 <https://bugs.openjdk.java.net/browse/JDK-8179327>
>>>> Webrev: http://cr.openjdk.java.net/~stuefe/webrevs/
>>>> 8179327-gtestLauncher-should-have-an-option-to-avoid-
>>>> primordial-thread-for-tests/webrev.00/webrev/
>>>>
>>>> This fix is motivated by the inability of the AIX openjdk vm to run on
>>>> the
>>>> primordial thread (see https://bugs.openjdk.java.net/browse/JDK-8171505
>>>> ),
>>>> which we unfortunately cannot do much about. This prevented us until now
>>>> to
>>>> run gtests successfully, because the gtestlauncher (in contrast to the
>>>> standard java launcher) invokes the VM on the primordial thread.
>>>>
>>>> To fix this, I gave the gtest launcher the ability to run the tests in an
>>>> own thread. Instead of sprinkling the source with "#ifdef _AIX", I added
>>>> a
>>>> command line option "-new-thread", which allows to start the tests in a
>>>> new
>>>> thread, and implemented this feature for all platforms (well, posix and
>>>> windows). On all platforms save AIX this is by default off, so nothing
>>>> changes. On AIX, tests will be default be executed in a new thread.
>>>>
>>>> Thanks for reviewing,
>>>>
>>>> Kind Regards, Thomas
>>>>
>>>>
>>
More information about the hotspot-dev
mailing list