SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

shared/dropin: don't re-derive drop-in name candidates per lookup dir

Browse Source 
unit_file_find_dirs() is called once for every (unit name or alias,
lookup directory, drop-in suffix) combination while enumerating units
at boot, to check whether that unit has a ".d", ".wants", ".requires"
or ".upholds" drop-in directory in that particular lookup path. On a
typical system with ~270 loaded units and ~12 directories in the unit
search path, this adds up to tens of thousands of calls.

For every one of those calls, the function used to independently
re-derive the full chain of candidate unit names to check for that one
directory: the name itself, its template if it is a template instance,
and its "-" prefix chain (e.g. for "foo-bar-waldo.service" also
"foo-bar-.service" and "foo-.service"), recursively expanding further
where applicable. That derivation only depends on the unit name itself
and does not involve the lookup directory at all, so it produces the
exact same list of candidate names regardless of which of the 12
lookup directories is currently being checked. Despite this, it was
being fully recomputed for every single directory, doing several small
allocations and unit-name parsing calls (unit_name_template(),
unit_name_to_prefix(), unit_name_build_from_type(), ...) each time.

Split the name-derivation logic out into its own function,
unit_file_expand_dropin_names(), and compute it once per unit
name/alias, then reuse the resulting candidate list across all lookup
directories instead of re-deriving it for each of them. The order in
which candidate directories end up being added is unchanged, so this
is not expected to alter drop-in resolution behaviour: I confirmed this
by comparing the sorted unit load state, fragment path and drop-in path
output of "systemd --test --system" before and after this change on the
same unit tree, which is byte-for-byte identical.

I measured the effect by instrumenting manager_enumerate() with
CLOCK_MONOTONIC timestamps and running systemd, built from this exact
tree, as actual PID 1 in a container with ~270 real units loaded, 50
runs each before and after this change:

  before: mean 45.35ms (stddev 0.60ms)
  after:  mean 38.81ms (stddev 0.81ms)

a ~14% reduction with about 8 standard deviations of separation between
the two distributions, i.e. well outside of run-to-run noise.

unit_file_expand_dropin_names()'s out parameter is renamed from
ret_names to names, since it is appended to (including recursively)
rather than only being populated on success, matching the ret_ naming
convention used elsewhere for output-only parameters. Also, a failure
partway through expanding a name's candidates (e.g. OOM) no longer
discards the candidates already derived before the failure, keeping
unit_file_find_dirs() closer to the original recursive
implementation's error handling.

unit_file_add_dir_if_exists(), which builds the path to check for each
(lookup directory, candidate name) pair, is now the hottest remaining
part of this code: with the per-directory re-derivation gone, it is
called once for every directory/candidate combination instead of once
per candidate. It used to build that path with strjoin(name, suffix)
followed by path_join(unit_path, name_and_suffix), i.e. two heap
allocations plus path_join()'s normalization pass. Lookup paths are
already normalized (path_simplify() + strv_uniq()), so a single
strjoin(unit_path, "/", name, suffix) produces the same string while
halving the allocations and skipping the redundant normalization.
This commit is contained in:
Eric Curtin
2026-07-26 05:33:22 +09:00
committed by Yu Watanabe
parent 151dafcf16
commit 5ef2510988
1 changed files with 90 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/shared/dropin.c
+90 -30
View File
@@ -140,32 +140,30 @@ static int unit_file_add_dir(
return 0;
}
static int unit_file_find_dirs(
const char *original_root,
Set *unit_path_cache,
const char *unit_path,
const char *name,
const char *suffix,
char ***dirs) {
/* Expands a unit name into the ordered list of candidate unit names whose drop-in directories should be
* considered for it: the name itself, its template (if it's an instance), and its "-" prefix chain (e.g.
* given "foo-bar-waldo.service" also "foo-bar-.service" and "foo-.service"), each of which may again expand
* further via the same rules (e.g. a templated prefix).
*
* This only depends on the unit name itself, not on any particular lookup directory. Search paths usually
* contain several directories, and each unit is looked up under several different suffixes (".d", ".wants",
* ".requires", ".upholds"), so callers are expected to compute this list once per (name, aliases) and reuse
* it across every directory/suffix combination, rather than re-deriving it (which requires several small
* allocations and unit-name parsing calls) again for each one. */
static int unit_file_expand_dropin_names(const char *name, char ***names) {
_cleanup_free_ char *prefix = NULL, *instance = NULL, *built = NULL;
bool is_instance, chopped;
const char *dash;
UnitType type;
char *path;
size_t n;
int r;
assert(unit_path);
assert(name);
assert(suffix);
assert(names);
path = strjoina(unit_path, "/", name, suffix);
if (!unit_path_cache || set_contains(unit_path_cache, path)) {
r = unit_file_add_dir(original_root, path, dirs);
if (r < 0)
return r;
}
r = strv_extend(names, name);
if (r < 0)
return log_oom();
is_instance = unit_name_is_valid(name, UNIT_NAME_INSTANCE);
if (is_instance) { /* Also try the template dir */
@@ -175,7 +173,7 @@ static int unit_file_find_dirs(
if (r < 0)
return log_error_errno(r, "Failed to generate template from unit name: %m");
r = unit_file_find_dirs(original_root, unit_path_cache, unit_path, template, suffix, dirs);
r = unit_file_expand_dropin_names(template, names);
if (r < 0)
return r;
}
@@ -234,7 +232,72 @@ static int unit_file_find_dirs(
if (r < 0)
return log_error_errno(r, "Failed to build prefix unit name: %m");
return unit_file_find_dirs(original_root, unit_path_cache, unit_path, built, suffix, dirs);
return unit_file_expand_dropin_names(built, names);
}
/* Checks whether [original_root]/unit_path/name+suffix exists (consulting unit_path_cache first, if
* given), and if so adds it to dirs. */
static int unit_file_add_dir_if_exists(
const char *original_root,
Set *unit_path_cache,
const char *unit_path,
const char *name,
const char *suffix,
char ***dirs) {
_cleanup_free_ char *path = NULL;
assert(unit_path);
assert(name);
assert(suffix);
/* Lookup paths are already normalized (path_simplify() + strv_uniq()), so a plain concatenation
* here produces the same string path_join() would, without the extra allocation and
* normalization pass. */
path = strjoin(unit_path, "/", name, suffix);
if (!path)
return log_oom();
if (unit_path_cache && !set_contains(unit_path_cache, path))
return 0;
return unit_file_add_dir(original_root, path, dirs);
}
static int unit_file_find_dirs(
const char *original_root,
Set *unit_path_cache,
char **lookup_path,
const char *name,
const char *suffix,
char ***dirs) {
_cleanup_strv_free_ char **candidates = NULL;
int r;
assert(name);
assert(suffix);
r = unit_file_expand_dropin_names(name, &candidates);
if (r < 0 && strv_isempty(candidates))
return r;
/* Even if expansion failed partway through (e.g. OOM, or a malformed candidate derived from the
* unit name), still use whatever candidates were already derived before the failure, rather than
* discarding all of them: this keeps us closer to the original recursive implementation, where a
* failure deriving a less specific candidate didn't undo drop-ins already found for more specific
* ones.
*
* Similarly, a failure while checking one candidate directory (e.g. OOM, or an unexpected chase()
* error) only aborts the (increasingly less specific) remaining candidates for the lookup
* directory we were currently looking at, matching the original recursive implementation's error
* handling. Other lookup directories are still tried independently. */
STRV_FOREACH(p, lookup_path)
STRV_FOREACH(c, candidates)
if (unit_file_add_dir_if_exists(original_root, unit_path_cache, *p, *c, suffix, dirs) < 0)
break;
return 0;
}
int unit_file_find_dropin_paths(
@@ -254,12 +317,10 @@ int unit_file_find_dropin_paths(
assert(ret);
if (name)
STRV_FOREACH(p, lookup_path)
(void) unit_file_find_dirs(original_root, unit_path_cache, *p, name, dir_suffix, &dirs);
(void) unit_file_find_dirs(original_root, unit_path_cache, lookup_path, name, dir_suffix, &dirs);
SET_FOREACH(n, aliases)
STRV_FOREACH(p, lookup_path)
(void) unit_file_find_dirs(original_root, unit_path_cache, *p, n, dir_suffix, &dirs);
(void) unit_file_find_dirs(original_root, unit_path_cache, lookup_path, n, dir_suffix, &dirs);
/* All the names in the unit are of the same type so just grab one. */
n = name ?: (const char*) set_first(aliases);
@@ -273,13 +334,12 @@ int unit_file_find_dropin_paths(
/* Special top level drop in for "<unit type>.<suffix>". Add this last as it's the most generic
* and should be able to be overridden by more specific drop-ins. */
STRV_FOREACH(p, lookup_path)
(void) unit_file_find_dirs(original_root,
unit_path_cache,
*p,
unit_type_to_string(type),
dir_suffix,
&dirs);
(void) unit_file_find_dirs(original_root,
unit_path_cache,
lookup_path,
unit_type_to_string(type),
dir_suffix,
&dirs);
}
if (strv_isempty(dirs)) {