Hmm, shouldn't we rather add ListUnitsByName() or so, that is like ListUnits() but allows you to pass in precisely which units you want to cover? It would be a lot like ListUnitsByPatterns(), except it would not resolve patterns, nor care about states, but just dump the entries for the names listed, but in the same format as ListUnits()?

@poettering ListUnits returns data which is loaded in systemd memory. GetUnitsStates works with units which are not in memory or have inactive state (and even not existing in the system). Of course I can add extra code into existing list_units_filtered function. It will be triggered only when patterns is not empty. But in this case we have patterns, not real unit names. So the solution would be to use patterns as a filter for unit_file_get_list and then loop through these unit files and execute manager_load_unit function for each of these units.

I was not quite sure whether I should implement such logic. Let me know what you think.

Well, the memory management model of systemd is really that we'll implicitly load all units that people request operations on. Hence, whether something is loaded or not is mostly a detail of the implementation – with one exception: ListUnits() only tells you about units currently loaded.

So, I think this should exposed as ListUnitsByName(). That said, there's probably no need to work this into list_units_filtered(), since what we loop around is quite different. That said, I think the code that's run inside the loop and ultimately adds the unit data into the dbus array could probably be shared, by splitting it out into a new funciton that is used by both.

@poettering just clarifying, do you mean we have to implement new method called ListUnitsByName, but not modification of the ListUnitsByPatterns? If so, should I try to retrieve units info from the memory first and if unit doesn't exists and the pattern is a valid unit name, try to use manager_load_unit? As a result there will be new shared function which will contain this loop (with minor modifications explained above).

Well, it's not about "having to" really. A convincing patch always wins. But I think that exposing this as ListUnitsByName and leaving ListUnitsByPatterns as it is is fully OK.

Always just use manager_load_unit(). It will load the unit if necessary, and otherwise just return the unit immediately.

And yeah, the second half of that loop you linked I'd probably turn into a function of its own, and reuse for this loop. I.e. the part where the data is pulled from the unit and written to the dbus message.

@poettering done.

Some tests on 500 inactive units:

$ time busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager ListUnitsByNames as 1000 hello@{1..1000}.service > /dev/null

real    0m0.204s
user    0m0.009s
sys     0m0.000s
$ time busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager ListUnitsByNames as 1000 hello@{1..1000}.service > /dev/null

real    0m0.178s
user    0m0.007s
sys     0m0.001s
$ time busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager ListUnitsByNames as 1000 hello@{1..1000}.service > /dev/null

real    0m0.397s
user    0m0.009s
sys     0m0.000s

and without the patch, using old org.freedesktop.DBus.Properties GetAll method:

$ time for i in {1..1000}; do busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/hello_40${i}_2eservice org.freedesktop.DBus.Properties GetAll "s" "" > /dev/null ;done 

real    0m11.483s
user    0m0.852s
sys     0m0.469s
$ time for i in {1..1000}; do busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/hello_40${i}_2eservice org.freedesktop.DBus.Properties GetAll "s" "" > /dev/null ;done 

real    0m12.631s
user    0m0.906s
sys     0m0.468s
$ time for i in {1..1000}; do busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/hello_40${i}_2eservice org.freedesktop.DBus.Properties GetAll "s" "" > /dev/null ;done 

real    0m13.253s
user    0m0.856s
sys     0m0.504s