RFR: 8319554: Select LogOutput* directly for stdout and stderr

[Johan Sjölen]

On Tue, 7 Nov 2023 17:16:07 GMT, Xin Liu <xliu at openjdk.org> wrote:

> This patch skips allocation on C heap and invoking jio_snprintf for stdout and stderr. 
> They are 2 predefined LogOuptuts and can't be deleted. 
> 
> We also check the return value of set_log_config in test_asynclog.cpp. if configuration fails,
> the test will be skipped. We expect tot fix the flaky failures(JDK-8309067).

Just a couple of style issues. Why do you expect this to fix the flaky failures? Is it specifically configuring the logging that fails?

src/hotspot/share/logging/logConfiguration.cpp line 513:

> 511:   } else if (0 == strcmp(outputstr, StderrLog->name())) { // stderr
> 512:     idx = 1;
> 513:     assert(find_output(outputstr) == idx, "sanity check");

This is the first time I see [Yoda conditions](https://en.wikipedia.org/wiki/Yoda_conditions) in HotSpot :-), maybe do `strcmp(...) == 0` instead?

test/hotspot/gtest/logging/test_asynclog.cpp line 259:

> 257:   testing::internal::CaptureStdout();
> 258:   fprintf(stdout, "header");
> 259:   if (set_log_config("stdout", "logging=debug")) {

Please invert this condition so that the main body of code doesn't have to be indented:


if(!set_log_confg(...)) {
  return;
}

Same with `write_to_file`.

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

PR Review: https://git.openjdk.org/jdk/pull/16543#pullrequestreview-1719855096
PR Review Comment: https://git.openjdk.org/jdk/pull/16543#discussion_r1386311770
PR Review Comment: https://git.openjdk.org/jdk/pull/16543#discussion_r1386309356