RFR: 8229517: Support for optional asynchronous/buffered logging [v11]

[Thomas Stuefe]

On Fri, 7 May 2021 08:10:56 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> test/hotspot/gtest/logging/test_asynclog.cpp line 44:
>> 
>>> 42: };
>>> 43: 
>>> 44: class AsyncLogTest : public LogTestFixture {
>> 
>> Unless you plan to add other code to this fixture I would prefer a regular RAII object like we do in hotspot, and naming it `AsyncModeMark`, and placing it at the start of each function instead. 
>> 
>> Reason: you can use EXPECT macros inside to test stuff and they fire as part of the test, its easier to debug, and it is common notation so hotspot devs know what this is without looking up rules about googletest fixtures.
>> 
>> But kudos for leaving clean state after your tests.
>
> I tried but I found it's  difficult not to use LogTestFixture here for async logging tests.
>  
> The constructor and destructor of LogTestFixture help me do the chores. 
> Without them, I can't use this set up function.
>   `set_log_config(TestLogFileName, "logging=debug");`

I see what you mean. I have thought about this some more. 

Unfortunately this requires you to be able to initialize and shutdown async logging at arbitrary times in the middle of VM life. Which normally would not be necessary. Not a big deal, but here is a slightly different proposal (and its just a proposal, I leave it up to you if you take this):

Instead of starting up and shutting down async logging at the start/end of a test (or, instead of the TEST_OTHER_VM proposal I did earlier), how about this:

1) Change your tests to not modify logging options. No fixture, no setup/teardown, no setup_logging(). Leave the tests passive: In the tests, you just react on those options (eg skipping AsyncLogTests if async logging is disabled)
2) Instead, you hand in all logging options you need from the outside via command line into the gtestLauncher. 
3) rename your tests to start with "Log..." like all other UL gtests. 
4) Then, add a gtest run configuration to the gtest jtreg wrapper: add a configuration with async=true and lots of logging. Limit on this command line the gtests to only run UL gtests ('--gtest_filter=Log*' )

This is actually very easy, I have done this for large page tests and metaspace test, among other things. See `test/hotspot/jtreg/gtest/LargePageGtests.java` or `test/hotspot/jtreg/gtest/MetaspaceGtests.java`. 

The advantage would be:
- no need to fiddle with log options in your tests, and to reinstate them at teardown
- no danger of accidentally disturbing other neighboring gtests, if yours is executed in parallel to others (we don't do this today but its possible).
- You execute all logging gtests - not only yours - with both async=false and async=true, and this is a better test coverage. This would be a nice stress test for async logging.

I understand if you don't want to do this now. This would be a fine future RFE (since actually, it should be done for all UL tests, not only yours).

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

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