RFR: 8293873: Centralize the initialization of UL [v2]
Thomas Stuefe
stuefe at openjdk.org
Fri Sep 23 14:05:18 UTC 2022
On Fri, 23 Sep 2022 10:38:21 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:
>> Hi,
>>
>> May I please have a review of this change? It moves around some of the initialization code of UL to one place (namely `LogConfiguration::initialize`). It does not change how UL works after VM start up, and it should not change how UL works after VM shut down, but it might change how UL works before VM start up. I don't believe that this causes any breakage.
>>
>> Tier 2 and 3 tests are currently running, all green so far.
>>
>> Some more context is available at https://bugs.openjdk.org/browse/JDK-8293873
>
> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
>
> Fix copyright years
Not a full review, since I'm about to go into vacation mode. But I like this, this is cleaner than before.
Not your patch, but the gtests made me think. Some of them are TEST, not TEST_VM, and I wonder if that's deliberate. TEST tests don't initialize the VM. Whether or not the VM infrastructure exists depends on whether a TEST_VM test happened to run before, so the tests depend on order of execution (which is annoying :-( )
If you run the LogTestSet tests isolated (--gtest_filter=LogTagSet_xxx), do they succeed?
Another thing to ponder is that the LogConfiguration gtests modify global state, AFAICS. But gtests should not have side effects. And they should be executable repeatedly (--gtest_repeat=xx). Therefore, these tests maybe should be TEST_OTHER_VM.
Cheers, Thomas
src/hotspot/share/logging/logFileStreamOutput.hpp line 77:
> 75: LogStdoutOutput() : LogFileStreamOutput(stdout) {
> 76: set_config_string("all=warning");
> 77: }
Since you are here :-)
Wonder whether `_config_string` and `_config_string_buffer_size` could be replaced by an inline `stringStream,` and then we could save the awkward string shuffling in `set_config_string()` and `add_to_config_string()`.
-------------
PR: https://git.openjdk.org/jdk/pull/10307
More information about the hotspot-runtime-dev
mailing list