core: Filter by unit name behind the D-Bus, instead on the client side #3142

kayrus · 2016-04-28T18:12:14Z

This PR improves systemd performance on the systems which have thousands of units. Resolves coreos/fleet#478 and partly obsoletes coreos/fleet#1418 (will create new patch with the new systemd dbus method soon)

Benchmarking:

test unit file:

[Service]
Type=oneshot
RemainAfterExit=true
ExecStart=/bin/true

cd /etc/systemd/system
for i in {1..1000}; do cp -p hello.service hello$i.service ; done
cp -p hello.service [email protected]
for i in {1..1000}; do systemctl start --no-block hello$i.service ; done
for i in {1..1000}; do systemctl start --no-block hello@$i.service ; done

Tests without the patch:

$ time busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager ListUnitFiles > /dev/null

real    0m0.934s
user    0m0.003s
sys     0m0.002s
$ time systemctl list-unit-files rsyncd.service --state=disabled > /dev/null

real    0m0.919s
user    0m0.001s
sys     0m0.001s
$ time for i in {1..1000}; do busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager ListUnitsFiltered as 0 > /dev/null; done

real    0m28.465s
user    0m16.550s
sys     0m1.015s
$ time for i in {1..1000}; do systemctl list-units hello@$i.service > /dev/null ; done

real    0m14.086s
user    0m5.496s
sys     0m1.118s
$ time for i in {1..1000}; do systemctl list-units hello@$i.service > /dev/null ; done

real    0m13.972s
user    0m5.482s
sys     0m1.014s
$ time for i in {1..1000}; do systemctl list-units hello@$i.service > /dev/null ; done

real    0m14.074s
user    0m5.411s
sys     0m1.140s
$ time for i in {1..1000}; do systemctl list-units hello$i.service > /dev/null ; done

real    0m14.021s
user    0m5.390s
sys     0m1.077s
$ time for i in {1..1000}; do systemctl list-units hello$i.service > /dev/null ; done

real    0m14.124s
user    0m5.359s
sys     0m1.129s
$ time for i in {1..1000}; do systemctl list-units hello$i.service > /dev/null ; done

real    0m14.125s
user    0m5.554s
sys     0m1.112s
======================HERE=I=USE=ONLY=10====================================
$ time for i in {1..10}; do systemctl list-unit-files hello$i.service > /dev/null ; done 

real    0m10.095s
user    0m0.022s
sys     0m0.008s
$ time for i in {1..10}; do systemctl list-unit-files hello$i.service > /dev/null ; done 

real    0m9.957s
user    0m0.018s
sys     0m0.005s
$ time for i in {1..10}; do systemctl list-unit-files hello$i.service > /dev/null ; done 

real    0m9.704s
user    0m0.016s
sys     0m0.010s

Tests with the patch:

$ time busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager ListUnitFilesFiltered asas 0 2 "hello1.service" "[email protected]" > /dev/null

real    0m0.005s
user    0m0.001s
sys     0m0.001s

$ time systemctl list-unit-files rsyncd.service --state=disabled > /dev/null

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

$ time for i in {1..1000}; do busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager ListUnitsFiltered asas 0 2 "hello$i.service" "hello@$i.service" > /dev/null; done

real    0m4.099s
user    0m0.602s
sys     0m0.772s
$ time for i in {1..1000}; do systemctl list-units hello@$i.service > /dev/null ; done

real    0m10.038s
user    0m0.597s
sys     0m0.712s
$ time for i in {1..1000}; do systemctl list-units hello@$i.service > /dev/null ; done

real    0m9.975s
user    0m0.579s
sys     0m0.726s
$ time for i in {1..1000}; do systemctl list-units hello@$i.service > /dev/null ; done

real    0m10.307s
user    0m0.623s
sys     0m0.735s
$ time for i in {1..1000}; do systemctl list-units hello$i.service > /dev/null ; done

real    0m9.768s
user    0m0.633s
sys     0m0.671s
$ time for i in {1..1000}; do systemctl list-units hello$i.service > /dev/null ; done

real    0m10.071s
user    0m0.612s
sys     0m0.695s
$ time for i in {1..1000}; do systemctl list-units hello$i.service > /dev/null ; done

real    0m9.976s
user    0m0.590s
sys     0m0.715s
======================HERE=I=USE=1000========================================
$ time for i in {1..1000}; do systemctl list-unit-files hello$i.service > /dev/null ; done 

real    0m11.263s
user    0m0.570s
sys     0m0.732s
$ time for i in {1..1000}; do systemctl list-unit-files hello$i.service > /dev/null ; done 

real    0m11.329s
user    0m0.566s
sys     0m0.715s
$ time for i in {1..1000}; do systemctl list-unit-files hello$i.service > /dev/null ; done 

real    0m11.697s
user    0m0.587s
sys     0m0.757s

P.S. Not sure about the existing method modification, most probably you'll suggest to create new dbus method.

=============================================================================

P.P.S. I've also noticed performance degradation in systemd (between v225 and master branch) when compared:

$ time busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager ListUnitsFiltered asas 0 2 "hello1.service" "[email protected]" > /dev/null

real    0m0.004s
user    0m0.002s
sys     0m0.000s
$ time systemctl -r list-units hello1.service [email protected] > /dev/null
real    0m0.011s
user    0m0.001s
sys     0m0.001s

Something takes extra 0m0.007s in systemd v225 had similar time comparing to busctl call

Similar tests as above but on systemd v225 (5 seconds in v225 vs 10 seconds in master branch):

$ time for i in {1..1000}; do busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager ListUnitsFiltered asas 0 1 "hello@$i.service" > /dev/null ; done

real    0m3.286s
user    0m0.234s
sys     0m0.355s
$ time for i in {1..1000}; do systemctl list-units hello$i.service > /dev/null ; done

real    0m5.506s
user    0m0.630s
sys     0m0.664s

keszybz · 2016-04-28T22:43:54Z

Please include the full description of the patch in the commit message, i.e. move the text from the PR to the commit.

This is something that I've thought about doing for a long time. It's reasonable to filter on the "server" side, because we double the work by doing it on client side.

Two high level changes will be needed to the patch though:

a new dbus method must be added, we cannot add arguments to the existing one
systemctl must have compatibility with systemd without the new method, i.e. it must fall back to calling the old method and filtering on the client side.

The patch looks nice though, it's much less code than I thought.

poettering · 2016-04-29T09:56:52Z

Yeah, looks conceptually OK. But this needs to be a new bus call, for compat reasons, we cannot simply alter the signature of existing bus calls. Call it ListUnitsWildcarded() or so...

kayrus · 2016-04-29T12:37:46Z

@keszybz @poettering updated PR

poettering · 2016-04-29T12:41:20Z

src/core/dbus-manager.c

        SD_BUS_METHOD("ResetFailed", NULL, NULL, method_reset_failed, SD_BUS_VTABLE_UNPRIVILEGED),
        SD_BUS_METHOD("ListUnits", NULL, "a(ssssssouso)", method_list_units, SD_BUS_VTABLE_UNPRIVILEGED),
        SD_BUS_METHOD("ListUnitsFiltered", "as", "a(ssssssouso)", method_list_units_filtered, SD_BUS_VTABLE_UNPRIVILEGED),
+        SD_BUS_METHOD("ListUnitsByStatesAndPatterns", "asas", "a(ssssssouso)", method_list_units_filtered_by_states_and_patterns, SD_BUS_VTABLE_UNPRIVILEGED),


The name sounds a bit cumbersome. Please just call this "ListUnitsByPatterns" or so...

poettering · 2016-04-29T12:47:25Z

ok, looks better, but still found some issues.

This commit improves systemd performance on the systems which have thousands of units.

kayrus · 2016-04-29T13:31:38Z

@poettering fixed

keszybz added pid1 systemctl reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Apr 28, 2016

kayrus force-pushed the kayrus/dbus_filter_units_master branch from 779e45b to 3119707 Compare April 29, 2016 12:36

kayrus changed the title ~~core: Filter by unit name behind the D-Bus, instead on client side~~ core: Filter by unit name behind the D-Bus, instead on the client side Apr 29, 2016

poettering reviewed Apr 29, 2016
View reviewed changes

core: Filter by unit name behind the D-Bus, instead on the client side

feec56f

This commit improves systemd performance on the systems which have thousands of units.

kayrus force-pushed the kayrus/dbus_filter_units_master branch from 3119707 to feec56f Compare April 29, 2016 13:31

poettering merged commit 313fe66 into systemd:master Apr 29, 2016

This was referenced Apr 29, 2016

dbus: add support for new ListUnits* methods coreos/go-systemd#150

Merged

core: added ListUnitsByNames dbus method #3182

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

core: Filter by unit name behind the D-Bus, instead on the client side #3142

core: Filter by unit name behind the D-Bus, instead on the client side #3142

Uh oh!

kayrus commented Apr 28, 2016 •

edited

Loading

Uh oh!

keszybz commented Apr 28, 2016

Uh oh!

poettering commented Apr 29, 2016

Uh oh!

kayrus commented Apr 29, 2016

Uh oh!

poettering Apr 29, 2016

Uh oh!

poettering commented Apr 29, 2016

Uh oh!

kayrus commented Apr 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Uh oh!

core: Filter by unit name behind the D-Bus, instead on the client side #3142

core: Filter by unit name behind the D-Bus, instead on the client side #3142

Uh oh!

Conversation

kayrus commented Apr 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keszybz commented Apr 28, 2016

Uh oh!

poettering commented Apr 29, 2016

Uh oh!

kayrus commented Apr 29, 2016

Uh oh!

poettering Apr 29, 2016

Choose a reason for hiding this comment

Uh oh!

poettering commented Apr 29, 2016

Uh oh!

kayrus commented Apr 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

kayrus commented Apr 28, 2016 •

edited

Loading