Add a Hashmap for enabled Units [IN PROGRESS] #1

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@davidstrauss

This patch adds a Hashmap object in the Manger for setting enabled Units, in
order to improve performance of the "is-enabled" subcommand of systemctl.

Signed-off-by: Rami Rosen ramirose@gmail.com

@ramirosen ramirosen Add a Hashmap for enabled Units.
This patch adds a Hashmap object in the Manger for setting enabled Units, in
order to improve performance of the "is-enabled" subcommand of systemctl.

Signed-off-by: Rami Rosen <ramirose@gmail.com>
1c126f8
@davidstrauss davidstrauss commented on the diff Mar 12, 2014
src/core/dbus-manager.c
@@ -1287,6 +1287,7 @@ static int method_get_unit_file_state(sd_bus *bus, sd_bus_message *message, void
UnitFileState state;
UnitFileScope scope;
int r;
+ bool disabled;
@davidstrauss
davidstrauss Mar 12, 2014

systemd doesn't have a concept of explicitly disabled services, except by masking, which is different from what we're optimizing right now. Because a service is disabled by lack of being enabled anywhere, we should use "enabled" as what we store, just to more accurately imply the underlying logic.

@davidstrauss davidstrauss commented on the diff Mar 12, 2014
src/core/dbus-manager.c
@@ -1301,11 +1302,14 @@ static int method_get_unit_file_state(sd_bus *bus, sd_bus_message *message, void
return r;
scope = m->running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM : UNIT_FILE_USER;
+ if (hashmap_get(m->enabled, name))
+ disabled = false;
+ else
+ disabled = true;
@davidstrauss
davidstrauss Mar 12, 2014

Can this be enabled = hashmap_get(m->enabled, name), after converting to using "enabled" instead of "disabled" in general?

@davidstrauss davidstrauss commented on the diff Mar 12, 2014
src/core/manager.h
@@ -68,6 +68,8 @@ struct Manager {
Hashmap *units; /* name string => Unit object n:1 */
Hashmap *jobs; /* job id => Job object 1:1 */
+ Hashmap *enabled; /* a hashmap for enabled Units */
@davidstrauss
davidstrauss Mar 12, 2014

Maybe a cache for enabled units? It's clear from the declaration that it's a hashmap.

@davidstrauss davidstrauss commented on the diff Mar 12, 2014
src/shared/install.c
@@ -1660,7 +1660,8 @@ int unit_file_get_default(
UnitFileState unit_file_get_state(
UnitFileScope scope,
const char *root_dir,
- const char *name) {
+ const char *name,
+ bool disable) {
@davidstrauss
davidstrauss Mar 12, 2014

The function prototype lists the parameter as "disabled" rather than "disable." Anyway, let's move to "enabled" or something more descriptive, like "is_enabled."

@davidstrauss davidstrauss commented on the diff Mar 12, 2014
src/shared/install.c
@@ -1721,11 +1722,8 @@ UnitFileState unit_file_get_state(
}
}
- r = find_symlinks_in_scope(scope, root_dir, name, &state);
- if (r < 0)
- return r;
- else if (r > 0)
- return state;
+ if (!disable)
+ return UNIT_FILE_ENABLED;
@davidstrauss
davidstrauss Mar 12, 2014

Won't this cause the function to just return a constant based on the argument provided rather than returning the actual state?

@davidstrauss davidstrauss deleted the enabled-cache-01 branch Mar 13, 2014
@davidstrauss davidstrauss pushed a commit that referenced this pull request Sep 12, 2014
@pfl pfl test-dhcp6-client: Fix option length
The whole DHCPv6 test message length was incorrectly used as the length
of DHCPv6 options causing the following bad memory access:

$ build/test-dhcp6-client
Assertion 'interface_index >= -1' failed at ../src/libsystemd-network/sd-dhcp6-client.c:129, function sd_dhcp6_client_set_index(). Ignoring.
=================================================================
==29135==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7fe204aa9148 at pc 0x7fe204a5958f bp 0x7fff3e47d470 sp 0x7fff3e47d460
READ of size 1 at 0x7fe204aa9148 thread T0
    #0 0x7fe204a5958e in option_parse_hdr ../src/libsystemd-network/dhcp6-option.c:145
    #1 0x7fe204a59884 in dhcp6_option_parse ../src/libsystemd-network/dhcp6-option.c:165
    #2 0x7fe204a4eb9c in test_advertise_option ../src/libsystemd-network/test-dhcp6-client.c:227
    #3 0x7fe204a51c58 in main ../src/libsystemd-network/test-dhcp6-client.c:584
    #4 0x7fe2031590df in __libc_start_main (/lib64/libc.so.6+0x200df)
    #5 0x7fe204a4cc5b (/home/test/systemd/build/test-dhcp6-client+0x25c5b)

0x7fe204aa9148 is located 2 bytes to the right of global variable 'msg_advertise' from '../src/libsystemd-network/test-dhcp6-client.c' (0x7fe204aa9080) of size 198
0x7fe204aa9148 is located 56 bytes to the left of global variable 'msg_reply' from '../src/libsystemd-network/test-dhcp6-client.c' (0x7fe204aa9180) of size 173
SUMMARY: AddressSanitizer: global-buffer-overflow ../src/libsystemd-network/dhcp6-option.c:145 option_parse_hdr
d182960
@davidstrauss davidstrauss pushed a commit that referenced this pull request Oct 16, 2014
@ssahani ssahani socket-proxyd: Unchecked return value from library
CID 1237543 (#1 of 1): Unchecked return value from library
(CHECKED_RETURN)
25dbe4f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment