RFR: 8298730: Refactor subsystem_file_line_contents and add docs and tests

Johan Sjölen jsjolen at openjdk.org
Wed Dec 14 10:40:40 UTC 2022


On Wed, 14 Dec 2022 10:18:32 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

> Hi!
> 
> Citing the ticket directly:
> 
> The function subsystem_file_line_contents does the simple job of parsing lines in a file, but does so in a fairly complex way. This function can be simplified. The function also has a surprising API, as you need to know that in one case the format string needs to take 2 specifiers, instead of 1. 
> 
> Some more context:
> 
> `subsystem_file_line_contents` either parses files that look like this:
> 
> 
> one_value
> 
> 
> Called as: `subsystem_file_line_contents(ctrl, fname, nullptr, "%s")`
> 
> Or like this:
> 
> 
> key1 val1
> key2 val2
> 
> 
> Called as:  `subsystem_file_line_contents(ctrl, fname, "key1", "%s %s")`
> 
> The API for the key/value case is changed to: `subsystem_file_line_contents(ctrl, fname, "key1", "%s")`. Note: `"%s"`, not `"%s %s"`.

Some preliminary comments (due to @jerboaa):

- There are already cgroup tests in ` test/hotspot/gtest/runtime/test_os_linux_cgroups.cpp`. They should be merged. But shouldn't the tests be placed so that they mirror the directory structure where the source code they're testing is?
-  Tests should be added that correspond to the usages of this function in the real code.

There are a lot of other improvements that can be made, but I'm leaving all of these to a future RFE :-).

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

PR: https://git.openjdk.org/jdk/pull/11667


More information about the hotspot-runtime-dev mailing list