RFR: 8252148: vmError::controlled_crash should be #ifdef ASSERT

[Coleen Phillimore]

On Fri, 11 Dec 2020 07:18:06 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Moved some unit tests from vmError into a gtest and moved controlled_crash code under #ifdef ASSERT rather than #ifndef PRODUCT since the tests that use this are @requires = vm.debug.  This change keeps the ErrorHandlerTest=n option and other handler test options, because there are some tests that use this more but moves them to develop options (they were notproduct).  There are some ErrorHandlerTest=n tests that do more thorough hs_err_pid.log file verification that remain.
>> 
>> Tested with tier1-3 with product and fastdebug builds.  Built with optimized.
>> 
>> This is for JDK 17.
>
> src/hotspot/share/utilities/vmError.cpp line 1800:
> 
>> 1798:     case 2: guarantee(how == 0, "test guarantee"); break;
>> 1799: 
>> 1800:     // The other cases are unused.
> 
> I like that you cut down on the cases. Most of them were obsolete or not working correctly. 
> 
> How about compacting the cases? And maybe use a real enum. But this could also be done in a separate RFE.

I thought it was kind of strange but then I didn't think it was worth the time to change it.  The command line processing doesn't have a way to pass enums through -XX:ErrorHandlerTest=plainSEGV or something like that. So in the end I didn't think it was something bad enough to change.

Answers above:  On my machine, the additional gtests add about 3 seconds to the run time where the gtests on the debug build take about a minute.  Since the gtests are thought to be a more efficient and direct testing mechanism, I don't think anyone's thought about ways to reduce the testing to account for performance.

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

PR: https://git.openjdk.java.net/jdk/pull/1723