-
Notifications
You must be signed in to change notification settings - Fork 400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC: perf(base): do not unconditionally process cmdline with each getcmdline
call
#2314
base: master
Are you sure you want to change the base?
Conversation
…ine` call At boot, every time a script calls a `getarg` function, the `getcmdline` function is called, which parses /etc/cmdline, /etc/cmdline.d/*.conf and /proc/cmdline. Although a call only takes a few ns, this is quite inefficient due to the large number of times a `getarg` function is called. Only to show this number, I added a warning every time `getcmdline` is called in two vms, one with a minimal setup and other using the NetworkManager to get internet connection at boot (in both cases the content returned by `getcmdline` is always the same, i.e., no hook script is adding new content at boot). These are the results: ``` localhost:/home/dev # journalctl -b | grep -c getcmdline 43 nm:/home/dev # journalctl -b | grep -c getcmdline 67 ``` This patch adds a new function `setcmdline`, which parses command line options, exports the `CMDLINE` variable (required by `dracut-util`), and persists its content in a temporary file, so that different processes can access it (e.g. systemd generators). With this change, if a hook script adds new content in /etc/cmdline.d at boot, it must explicitly call `setcmdline` to update the content of the `CMDLINE` variable. Let's compare the performance when calling 67 times the old version and the new one (with the new version, `setcmdline` is called only once, no cmdline changes at boot): ``` sh-5.2# hyperfine -S'bash --norc' '. /lib/dracut-lib.sh ; for ((i=1;i<=67;i++)); do getcmdline_v0; done' '. /lib/dracut-lib.sh ; for ((i=1;i<=67;i++)); do getcmdline_v1; done' Benchmark 1: . /lib/dracut-lib.sh ; for ((i=1;i<=67;i++)); do getcmdline_v0; done Time (mean ± σ): 4.7 ms ± 0.6 ms [User: 3.9 ms, System: 1.1 ms] Range (min … max): 4.3 ms … 11.6 ms 380 runs Warning: Command took less than 5 ms to complete. Note that the results might be inaccurate because hyperfine can not calibrate the shell startup time much more precise than this limit. You can try to use the `-N`/`--shell=none` option to disable the shell completely. Warning: The first benchmarking run for this command was significantly slower than the rest (6.6 ms). This could be caused by (filesystem) caches that were not filled until after the first run. You should consider using the '--warmup' option to fill those caches before the actual benchmark. Alternatively, use the '--prepare' option to clear the caches before each timing run. Benchmark 2: . /lib/dracut-lib.sh ; for ((i=1;i<=67;i++)); do getcmdline_v1; done Time (mean ± σ): 1.1 ms ± 0.3 ms [User: 1.2 ms, System: 0.2 ms] Range (min … max): 0.8 ms … 2.4 ms 991 runs Warning: Command took less than 5 ms to complete. Note that the results might be inaccurate because hyperfine can not calibrate the shell startup time much more precise than this limit. You can try to use the `-N`/`--shell=none` option to disable the shell completely. Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options. Summary '. /lib/dracut-lib.sh ; for ((i=1;i<=67;i++)); do getcmdline_v1; done' ran 4.44 ± 1.29 times faster than '. /lib/dracut-lib.sh ; for ((i=1;i<=67;i++)); do getcmdline_v0; done' ``` Reported-by: Martin Wilck <[email protected]>
Just a thought - can we avoid creating some of the /etc/cmdline.d files ? As an example change
to
Would this break out-of-tree dracut modules ? |
We could, but I don't know if it's a good idea. If we remove these conf files, it would be more difficult to debug from the emergency shell which module adds command line parameters.
AFAIK the only documented use of /etc/cmdline.d to inject custom command line options is during the build process, not dynamically at boot. If an external dracut module is doing this (e.g. openSUSE kdump), then yes, it will need to adapt its code. But I think this should not be an obstacle to fix this inefficient code. |
This will also clean up debug xtrace logs. |
As I understand the measurement this PR has the potential of saving 4 ms on an average boot.
In other PR reviews lack of documentation did not necessary allow for breaking compatibility without a depreciation process - e.g. #2104 (comment) . It sounds like this PR would need to come with a new "supported" POSIX shell interface ( Given that out-of-tree modules need this functionality as well, I am not sure if the benefit of saving single digit ms outweighs the new set of problems in the current form of the PR.
Good point, but you're not optimizing for the "emergency shell" case. Maybe instead of deleting them you can move them to to some kind of "processed files" directory for "emergency shell" case. I am thinking that maybe there is a way to keep compatibility with the current |
This issue is being marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. If this is still an issue in the latest release of Dracut and you would like to keep it open please comment on this issue within the next 7 days. Thank you for your contributions. |
This issue is being marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. If this is still an issue in the latest release of Dracut and you would like to keep it open please comment on this issue within the next 7 days. Thank you for your contributions. |
This issue is being marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. If this is still an issue in the latest release of Dracut and you would like to keep it open please comment on this issue within the next 7 days. Thank you for your contributions. |
At boot, every time a script calls a
getarg
function, thegetcmdline
function is called, which parses /etc/cmdline, /etc/cmdline.d/*.conf and /proc/cmdline.Although a call only takes a few ns, this is quite inefficient due to the large number of times a
getarg
function is called. Only to show this number, I added a warning every timegetcmdline
is called in two vms, one with a minimal setup and other using the NetworkManager to get internet connection at boot (in both cases the content returned bygetcmdline
is always the same, i.e., no hook script is adding new content at boot). These are the results:This patch adds a new function
setcmdline
, which parses command line options, exports theCMDLINE
variable (required bydracut-util
), and persists its content in a temporary file, so that different processes can access it (e.g. systemd generators).With this change, if a hook script adds new content in /etc/cmdline.d at boot, it must explicitly call
setcmdline
to update the content of theCMDLINE
variable.Let's compare the performance when calling 67 times the old version and the new one (with the new version,
setcmdline
is called only once, no cmdline changes at boot):Checklist