Re: [systemd-devel] [PATCH, REVIEW] Added unit enabled-context cache to improve performance w/ many units.
On Tue, Oct 07, 2014 at 11:08:13PM -0700, Ken Sedgwick wrote: Resubmitting using git format-patch, git imap-send ... no code changes. Hi, thank you for working on systemd, it is appreciated. Nevertheless, this patch is hard to apply for a couple of reasons. First, it does not apply: it is line broken and base64 encoded. The encoding is not a problem, but I'd simply fix the lines otherwise, and with base64 encoding it is a hassle. Please, try to send it to yourself, save the mail and apply with git am. (You wrote that you send it with git-send-email, but this doesn't seem possible.) Second, please structure your commit messages as - a short line (an empty line) - a paragraph (en empty line) - ... And describe what the patch does, so the first (short) line should be a title like 'tests: add test for ...' and then the paragraphs following that should give an overview of the *reasons* for the change, and describe the new state if necessary. In case of a test that is fairly obvious, so a long description is not necessary, but at least write one sentence about what you are testing. The first line is short because it is shown in gitk, git log --oneline, etc, where there's no space for a sentence. Third, you sometimes included a changelog of what changed between patch versions, but not always. Please do this in case of a big patch like this. But do not include details like 'resent using git format-patch...' in this, because this is not something that would be interesting for a person reading the git log. Fourth, when sending multiple versions of multiple patches please either number them by version, or use --in-reply-to to attach them at the end of an existing thread, and if possible do both. Also, not strictly necessary, but if the patch was split (as it originally was) into two parts, the first adding tests, and the second one changing functionality, it would be much easier to review, at least for me. Fifth, please describe the patch that changes the functionality in detail: your assumptions, old behaviour, new behaviour, etc. This area of the code has had a long series of mistakes and broken assumptions. So, please resend, I'll try to review and apply. Zbyszek --- .gitignore | 1 + Makefile.am| 44 +++- src/core/dbus-manager.c| 4 +- src/core/manager.c | 6 + src/core/manager.h | 2 + src/core/unit.c| 2 +- src/shared/install.c | 268 + src/shared/install.h | 16 +- src/systemctl/systemctl.c | 4 +- src/test/test-enabled.c| 151 src/test/test-install.c| 87 --- src/test/test-unit-file.c | 2 +- .../etc/systemd/system/masked.service | 1 + .../etc/systemd/system/maskedstatic.service| 1 + .../etc/systemd/system/some.target | 11 + .../system/some.target.wants/aliased.service | 1 + .../system/some.target.wants/also_masked.service | 1 + .../system/some.target.wants/another.service | 1 + .../system/some.target.wants/different.service | 1 + .../system/some.target.wants/masked.service| 1 + .../some.target.wants/templating@four.service | 1 + .../some.target.wants/templating@one.service | 1 + .../some.target.wants/templating@three.service | 1 + .../some.target.wants/templating@two.service | 1 + .../run/systemd/system/maskedruntime.service | 1 + .../run/systemd/system/maskedruntimestatic.service | 1 + .../run/systemd/system/other.target| 14 ++ .../system/other.target.wants/runtime.service | 1 + .../usr/lib/systemd/system/another.service | 9 + .../usr/lib/systemd/system/disabled.service| 9 + .../usr/lib/systemd/system/invalid.service | 1 + .../usr/lib/systemd/system/masked.service | 9 + .../usr/lib/systemd/system/maskedruntime.service | 9 + .../lib/systemd/system/maskedruntimestatic.service | 6 + .../usr/lib/systemd/system/maskedstatic.service| 6 + .../usr/lib/systemd/system/runtime.service | 9 + .../usr/lib/systemd/system/static.service | 6 + .../usr/lib/systemd/system/templating@.service | 9 + .../lib/systemd/system/templating@three.service| 9 + .../usr/lib/systemd/system/templating@two.service | 9 + .../usr/lib/systemd/system/unique.service | 9 + 41 files changed, 626 insertions(+), 100 deletions(-) create mode 100644 src/test/test-enabled.c create mode 12 test/test-enabled-root/etc/systemd/system/masked.service create mode 12
Re: [systemd-devel] [PATCH, REVIEW] Added unit enabled-context cache to improve performance w/ many units.
On Tue, 07.10.14 23:08, Ken Sedgwick (ksedg...@bonsai.com) wrote: Resubmitting using git format-patch, git imap-send ... no code changes. Patch is line-broken! If nothing else works, simply attach the git-formatted patch. It's really hard following the patch with all those broken lines! Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH, REVIEW] Added unit enabled-context cache to improve performance w/ many units.
Resubmitting using git format-patch, git imap-send ... no code changes. --- .gitignore | 1 + Makefile.am| 44 +++- src/core/dbus-manager.c| 4 +- src/core/manager.c | 6 + src/core/manager.h | 2 + src/core/unit.c| 2 +- src/shared/install.c | 268 + src/shared/install.h | 16 +- src/systemctl/systemctl.c | 4 +- src/test/test-enabled.c| 151 src/test/test-install.c| 87 --- src/test/test-unit-file.c | 2 +- .../etc/systemd/system/masked.service | 1 + .../etc/systemd/system/maskedstatic.service| 1 + .../etc/systemd/system/some.target | 11 + .../system/some.target.wants/aliased.service | 1 + .../system/some.target.wants/also_masked.service | 1 + .../system/some.target.wants/another.service | 1 + .../system/some.target.wants/different.service | 1 + .../system/some.target.wants/masked.service| 1 + .../some.target.wants/templating@four.service | 1 + .../some.target.wants/templating@one.service | 1 + .../some.target.wants/templating@three.service | 1 + .../some.target.wants/templating@two.service | 1 + .../run/systemd/system/maskedruntime.service | 1 + .../run/systemd/system/maskedruntimestatic.service | 1 + .../run/systemd/system/other.target| 14 ++ .../system/other.target.wants/runtime.service | 1 + .../usr/lib/systemd/system/another.service | 9 + .../usr/lib/systemd/system/disabled.service| 9 + .../usr/lib/systemd/system/invalid.service | 1 + .../usr/lib/systemd/system/masked.service | 9 + .../usr/lib/systemd/system/maskedruntime.service | 9 + .../lib/systemd/system/maskedruntimestatic.service | 6 + .../usr/lib/systemd/system/maskedstatic.service| 6 + .../usr/lib/systemd/system/runtime.service | 9 + .../usr/lib/systemd/system/static.service | 6 + .../usr/lib/systemd/system/templating@.service | 9 + .../lib/systemd/system/templating@three.service| 9 + .../usr/lib/systemd/system/templating@two.service | 9 + .../usr/lib/systemd/system/unique.service | 9 + 41 files changed, 626 insertions(+), 100 deletions(-) create mode 100644 src/test/test-enabled.c create mode 12 test/test-enabled-root/etc/systemd/system/masked.service create mode 12 test/test-enabled-root/etc/systemd/system/maskedstatic.service create mode 100644 test/test-enabled-root/etc/systemd/system/some.target create mode 12 test/test-enabled-root/etc/systemd/system/some.target.wants/aliased.service create mode 12 test/test-enabled-root/etc/systemd/system/some.target.wants/also_masked.service create mode 12 test/test-enabled-root/etc/systemd/system/some.target.wants/another.service create mode 12 test/test-enabled-root/etc/systemd/system/some.target.wants/different.service create mode 12 test/test-enabled-root/etc/systemd/system/some.target.wants/masked.service create mode 12 test/test-enabled-root/etc/systemd/system/some.target.wants/templating@four.service create mode 12 test/test-enabled-root/etc/systemd/system/some.target.wants/templating@one.service create mode 12 test/test-enabled-root/etc/systemd/system/some.target.wants/templating@three.service create mode 12 test/test-enabled-root/etc/systemd/system/some.target.wants/templating@two.service create mode 12 test/test-enabled-root/run/systemd/system/maskedruntime.service create mode 12 test/test-enabled-root/run/systemd/system/maskedruntimestatic.service create mode 100644 test/test-enabled-root/run/systemd/system/other.target create mode 12 test/test-enabled-root/run/systemd/system/other.target.wants/runtime.service create mode 100644 test/test-enabled-root/usr/lib/systemd/system/another.service create mode 100644 test/test-enabled-root/usr/lib/systemd/system/disabled.service create mode 100644 test/test-enabled-root/usr/lib/systemd/system/invalid.service create mode 100644 test/test-enabled-root/usr/lib/systemd/system/masked.service create mode 100644 test/test-enabled-root/usr/lib/systemd/system/maskedruntime.service create mode 100644 test/test-enabled-root/usr/lib/systemd/system/maskedruntimestatic.service create mode 100644 test/test-enabled-root/usr/lib/systemd/system/maskedstatic.service create mode 100644 test/test-enabled-root/usr/lib/systemd/system/runtime.service create mode 100644 test/test-enabled-root/usr/lib/systemd/system/static.service create mode 100644
[systemd-devel] [PATCH, REVIEW] Added unit enabled-context cache to improve performance w/ many units.
The attached patch adds an EnabledContext cache so systems with 1000s of
units do not suffer
O(N^2) performance when determining unit state. The test-enabled unit test
(added to master under other patch)
is used to confirm that the returned states are the same as the current
master.
Please review.
Many thanks in advance,
Ken
--
Ken Sedgwick
Bonsai Software, Inc.
http://www.bonsai.com/ken/
(510) 269-7334
k...@bonsai.com
Public Key: http://www.bonsai.com/ken/ken.asc
GPG Fingerprint: 851E 3B07 E586 0843 9434 5CC7 4033 3B9B 3F3F 9640
diff --git a/.gitignore b/.gitignore
index f119b57..97b2b2b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -173,6 +173,7 @@
/test-icmp6-rs
/test-ellipsize
/test-engine
+/test-enabled
/test-env-replace
/test-event
/test-fdset
diff --git a/Makefile.am b/Makefile.am
index e52db17..7d4f2f5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1358,7 +1358,8 @@ tests += \
test-ratelimit \
test-condition-util \
test-uid-range \
- test-bus-policy
+ test-bus-policy \
+ test-enabled
EXTRA_DIST += \
test/a.service \
@@ -1394,8 +1395,36 @@ EXTRA_DIST += \
test/bus-policy/hello.conf \
test/bus-policy/methods.conf \
test/bus-policy/ownerships.conf \
- test/bus-policy/signals.conf
-
+ test/bus-policy/signals.conf \
+ test/test-enabled-root/usr/lib/systemd/system/masked.service \
+ test/test-enabled-root/usr/lib/systemd/system/runtime.service \
+ test/test-enabled-root/usr/lib/systemd/system/maskedruntime.service \
+ test/test-enabled-root/usr/lib/systemd/system/another.service \
+ test/test-enabled-root/usr/lib/systemd/system/templating@three.service \
+ test/test-enabled-root/usr/lib/systemd/system/maskedstatic.service \
+ test/test-enabled-root/usr/lib/systemd/system/invalid.service \
+ test/test-enabled-root/usr/lib/systemd/system/disabled.service \
+ test/test-enabled-root/usr/lib/systemd/system/templating@two.service \
+ test/test-enabled-root/usr/lib/systemd/system/unique.service \
+ test/test-enabled-root/usr/lib/systemd/system/templating@.service \
+ test/test-enabled-root/usr/lib/systemd/system/static.service \
+ test/test-enabled-root/usr/lib/systemd/system/maskedruntimestatic.service \
+ test/test-enabled-root/run/systemd/system/other.target.wants/runtime.service \
+ test/test-enabled-root/run/systemd/system/maskedruntime.service \
+ test/test-enabled-root/run/systemd/system/other.target \
+ test/test-enabled-root/run/systemd/system/maskedruntimestatic.service \
+ test/test-enabled-root/etc/systemd/system/masked.service \
+ test/test-enabled-root/etc/systemd/system/some.target.wants/masked.service \
+ test/test-enabled-root/etc/systemd/system/some.target.wants/another.service \
+ test/test-enabled-root/etc/systemd/system/some.target.wants/templating@three.service \
+ test/test-enabled-root/etc/systemd/system/some.target.wants/templating@one.service \
+ test/test-enabled-root/etc/systemd/system/some.target.wants/templating@two.service \
+ test/test-enabled-root/etc/systemd/system/some.target.wants/templating@four.service \
+ test/test-enabled-root/etc/systemd/system/some.target.wants/also_masked.service \
+ test/test-enabled-root/etc/systemd/system/some.target.wants/different.service \
+ test/test-enabled-root/etc/systemd/system/some.target.wants/aliased.service \
+ test/test-enabled-root/etc/systemd/system/maskedstatic.service \
+ test/test-enabled-root/etc/systemd/system/some.target
EXTRA_DIST += \
src/test/test-helper.h
@@ -1782,6 +1811,15 @@ test_install_LDADD = \
libsystemd-shared.la \
libsystemd-internal.la
+test_enabled_SOURCES = \
+ src/test/test-enabled.c
+
+test_enabled_LDADD = \
+ libsystemd-units.la \
+ libsystemd-label.la \
+ libsystemd-shared.la \
+ libsystemd-internal.la
+
test_watchdog_SOURCES = \
src/test/test-watchdog.c
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 533ce43..8dcd552 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1403,7 +1403,7 @@ static int method_list_unit_files(sd_bus *bus, sd_bus_message *message, void *us
if (!h)
return -ENOMEM;
-r = unit_file_get_list(m-running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM : UNIT_FILE_USER, NULL, h);
+r = unit_file_get_list(m-running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM : UNIT_FILE_USER, NULL, h, m-enabled);
if (r 0)
goto fail;
@@ -1454,7 +1454,7 @@ static int method_get_unit_file_state(sd_bus *bus, sd_bus_message *message, void
scope = m-running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM : UNIT_FILE_USER;
-state = unit_file_get_state(scope, NULL, name);
+state = unit_file_get_state(scope, NULL, name, m-enabled);
if (state 0)
return state;
diff --git a/src/core/manager.c b/src/core/manager.c
index e0c1cd1..c9aef42 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -465,6 +465,10 @@ int manager_new(SystemdRunningAs running_as, bool test_run, Manager **_m) {
if (r 0)
goto
