From 7d17cfbc46306a106dbda0f3e92fbc0792d1e9e9 Mon Sep 17 00:00:00 2001 From: Michal Schmidt Date: Sun, 15 Jan 2012 10:53:49 +0100 Subject: [PATCH] unit: reduce heap usage for unit objects The storage of the unit objects on the heap is currently not very efficient. For every unit object we allocate a chunk of memory as large as the biggest unit type, although there are significant differences in the units' real requirements. pahole shows the following sizes of structs: 488 Target 496 Snapshot 512 Device 528 Path 560 Timer 576 Automount 1080 Socket 1160 Swap 1168 Service 1280 Mount Usually there aren't many targets or snapshots in the system, but Device is one of the most common unit types and for every one we waste 1280 - 512 = 768 bytes. Fix it by allocating only the right amount for the given unit type. On my machine (x86_64, with 39 LVM volumes) this decreases systemd's USS (unique set size) by more than 300 KB. --- src/automount.c | 1 + src/device.c | 7 +++++-- src/manager.c | 17 ++++++++++++----- src/mount.c | 13 +++++++++---- src/path.c | 1 + src/service.c | 1 + src/snapshot.c | 1 + src/socket.c | 1 + src/swap.c | 13 +++++++++---- src/target.c | 1 + src/timer.c | 1 + src/unit.c | 9 ++++++--- src/unit.h | 5 ++++- 13 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/automount.c b/src/automount.c index b70f8b4684..c4191f6706 100644 --- a/src/automount.c +++ b/src/automount.c @@ -836,6 +836,7 @@ DEFINE_STRING_TABLE_LOOKUP(automount_state, AutomountState); const UnitVTable automount_vtable = { .suffix = ".automount", + .object_size = sizeof(Automount), .sections = "Unit\0" "Automount\0" diff --git a/src/device.c b/src/device.c index bffeca0d10..64665a8b87 100644 --- a/src/device.c +++ b/src/device.c @@ -198,10 +198,12 @@ static int device_update_unit(Manager *m, struct udev_device *dev, const char *p if (!u) { delete = true; - if (!(u = unit_new(m))) + u = unit_new(m, sizeof(Device)); + if (!u) return -ENOMEM; - if ((r = device_add_escaped_name(u, path)) < 0) + r = device_add_escaped_name(u, path); + if (r < 0) goto fail; unit_add_to_load_queue(u); @@ -583,6 +585,7 @@ DEFINE_STRING_TABLE_LOOKUP(device_state, DeviceState); const UnitVTable device_vtable = { .suffix = ".device", + .object_size = sizeof(Device), .sections = "Unit\0" "Device\0" diff --git a/src/manager.c b/src/manager.c index a549209ca6..683197396f 100644 --- a/src/manager.c +++ b/src/manager.c @@ -1795,6 +1795,7 @@ unsigned manager_dispatch_load_queue(Manager *m) { int manager_load_unit_prepare(Manager *m, const char *name, const char *path, DBusError *e, Unit **_ret) { Unit *ret; + UnitType t; int r; assert(m); @@ -1811,24 +1812,30 @@ int manager_load_unit_prepare(Manager *m, const char *name, const char *path, DB if (!name) name = file_name_from_path(path); - if (!unit_name_is_valid(name, false)) { + t = unit_name_to_type(name); + + if (t == _UNIT_TYPE_INVALID || !unit_name_is_valid_no_type(name, false)) { dbus_set_error(e, BUS_ERROR_INVALID_NAME, "Unit name %s is not valid.", name); return -EINVAL; } - if ((ret = manager_get_unit(m, name))) { + ret = manager_get_unit(m, name); + if (ret) { *_ret = ret; return 1; } - if (!(ret = unit_new(m))) + ret = unit_new(m, unit_vtable[t]->object_size); + if (!ret) return -ENOMEM; - if (path) - if (!(ret->meta.fragment_path = strdup(path))) { + if (path) { + ret->meta.fragment_path = strdup(path); + if (!ret->meta.fragment_path) { unit_free(ret); return -ENOMEM; } + } if ((r = unit_add_name(ret, name)) < 0) { unit_free(ret); diff --git a/src/mount.c b/src/mount.c index 12c0710f1f..f94c0eb278 100644 --- a/src/mount.c +++ b/src/mount.c @@ -1395,13 +1395,16 @@ static int mount_add_one( if (!is_path(where)) return 0; - if (!(e = unit_name_from_path(where, ".mount"))) + e = unit_name_from_path(where, ".mount"); + if (!e) return -ENOMEM; - if (!(u = manager_get_unit(m, e))) { + u = manager_get_unit(m, e); + if (!u) { delete = true; - if (!(u = unit_new(m))) { + u = unit_new(m, sizeof(Mount)); + if (!u) { free(e); return -ENOMEM; } @@ -1412,7 +1415,8 @@ static int mount_add_one( if (r < 0) goto fail; - if (!(MOUNT(u)->where = strdup(where))) { + MOUNT(u)->where = strdup(where); + if (!MOUNT(u)->where) { r = -ENOMEM; goto fail; } @@ -1837,6 +1841,7 @@ DEFINE_STRING_TABLE_LOOKUP(mount_exec_command, MountExecCommand); const UnitVTable mount_vtable = { .suffix = ".mount", + .object_size = sizeof(Mount), .sections = "Unit\0" "Mount\0" diff --git a/src/path.c b/src/path.c index 957af05c7e..ae5052aeab 100644 --- a/src/path.c +++ b/src/path.c @@ -717,6 +717,7 @@ DEFINE_STRING_TABLE_LOOKUP(path_type, PathType); const UnitVTable path_vtable = { .suffix = ".path", + .object_size = sizeof(Path), .sections = "Unit\0" "Path\0" diff --git a/src/service.c b/src/service.c index 113be34ad0..6ce6b19db9 100644 --- a/src/service.c +++ b/src/service.c @@ -3553,6 +3553,7 @@ DEFINE_STRING_TABLE_LOOKUP(notify_access, NotifyAccess); const UnitVTable service_vtable = { .suffix = ".service", + .object_size = sizeof(Service), .sections = "Unit\0" "Service\0" diff --git a/src/snapshot.c b/src/snapshot.c index 270dc4f252..161629d983 100644 --- a/src/snapshot.c +++ b/src/snapshot.c @@ -282,6 +282,7 @@ DEFINE_STRING_TABLE_LOOKUP(snapshot_state, SnapshotState); const UnitVTable snapshot_vtable = { .suffix = ".snapshot", + .object_size = sizeof(Snapshot), .no_alias = true, .no_instances = true, diff --git a/src/socket.c b/src/socket.c index 7034436be0..1a245aa872 100644 --- a/src/socket.c +++ b/src/socket.c @@ -2114,6 +2114,7 @@ DEFINE_STRING_TABLE_LOOKUP(socket_exec_command, SocketExecCommand); const UnitVTable socket_vtable = { .suffix = ".socket", + .object_size = sizeof(Socket), .sections = "Unit\0" "Socket\0" diff --git a/src/swap.c b/src/swap.c index 4fa30a3e32..202c4e6625 100644 --- a/src/swap.c +++ b/src/swap.c @@ -334,7 +334,8 @@ int swap_add_one( assert(m); assert(what); - if (!(e = unit_name_from_path(what, ".swap"))) + e = unit_name_from_path(what, ".swap"); + if (!e) return -ENOMEM; u = manager_get_unit(m, e); @@ -348,15 +349,18 @@ int swap_add_one( if (!u) { delete = true; - if (!(u = unit_new(m))) { + u = unit_new(m, sizeof(Swap)); + if (!u) { free(e); return -ENOMEM; } - if ((r = unit_add_name(u, e)) < 0) + r = unit_add_name(u, e); + if (r < 0) goto fail; - if (!(SWAP(u)->what = strdup(what))) { + SWAP(u)->what = strdup(what); + if (!SWAP(u)->what) { r = -ENOMEM; goto fail; } @@ -1341,6 +1345,7 @@ DEFINE_STRING_TABLE_LOOKUP(swap_exec_command, SwapExecCommand); const UnitVTable swap_vtable = { .suffix = ".swap", + .object_size = sizeof(Swap), .sections = "Unit\0" "Swap\0" diff --git a/src/target.c b/src/target.c index 340e9907f0..b774cfbc33 100644 --- a/src/target.c +++ b/src/target.c @@ -199,6 +199,7 @@ DEFINE_STRING_TABLE_LOOKUP(target_state, TargetState); const UnitVTable target_vtable = { .suffix = ".target", + .object_size = sizeof(Target), .sections = "Unit\0" "Target\0" diff --git a/src/timer.c b/src/timer.c index d127a11743..87adb29e7a 100644 --- a/src/timer.c +++ b/src/timer.c @@ -468,6 +468,7 @@ DEFINE_STRING_TABLE_LOOKUP(timer_base, TimerBase); const UnitVTable timer_vtable = { .suffix = ".timer", + .object_size = sizeof(Timer), .sections = "Unit\0" "Timer\0" diff --git a/src/unit.c b/src/unit.c index 1fbfb1dea5..fa3c2649b3 100644 --- a/src/unit.c +++ b/src/unit.c @@ -57,15 +57,18 @@ const UnitVTable * const unit_vtable[_UNIT_TYPE_MAX] = { [UNIT_PATH] = &path_vtable }; -Unit *unit_new(Manager *m) { +Unit *unit_new(Manager *m, size_t size) { Unit *u; assert(m); + assert(size >= sizeof(Meta)); - if (!(u = new0(Unit, 1))) + u = malloc0(size); + if (!u) return NULL; - if (!(u->meta.names = set_new(string_hash_func, string_compare_func))) { + u->meta.names = set_new(string_hash_func, string_compare_func); + if (!u->meta.names) { free(u); return NULL; } diff --git a/src/unit.h b/src/unit.h index 19314d6fbd..626bdc43af 100644 --- a/src/unit.h +++ b/src/unit.h @@ -286,6 +286,9 @@ union Unit { struct UnitVTable { const char *suffix; + /* How much memory does an object of this unit type need */ + size_t object_size; + /* Config file sections this unit type understands, separated * by NUL chars */ const char *sections; @@ -435,7 +438,7 @@ DEFINE_CAST(SNAPSHOT, Snapshot); DEFINE_CAST(SWAP, Swap); DEFINE_CAST(PATH, Path); -Unit *unit_new(Manager *m); +Unit *unit_new(Manager *m, size_t size); void unit_free(Unit *u); int unit_add_name(Unit *u, const char *name);