shared/exit-status: use Bitmap instead of Sets

I opted to embed the Bitmap structure directly in the ExitStatusSet.
This means that memory usage is a bit higher for units which don't define
this setting:

Service changes:
        /* size: 2720, cachelines: 43, members: 73 */
        /* sum members: 2680, holes: 9, sum holes: 39 */
        /* sum bitfield members: 7 bits, bit holes: 1, sum bit holes: 1 bits */
        /* last cacheline: 32 bytes */

        /* size: 2816, cachelines: 44, members: 73 */
        /* sum members: 2776, holes: 9, sum holes: 39 */
        /* sum bitfield members: 7 bits, bit holes: 1, sum bit holes: 1 bits */

But this way the code is simpler and we do less pointer chasing.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2019-07-29 15:54:53 +02:00
parent 8594c8a552
commit 23d5dd1687
7 changed files with 38 additions and 58 deletions
+10 -22
View File
@@ -39,9 +39,9 @@ static int property_get_exit_status_set(
void *userdata,
sd_bus_error *error) {
ExitStatusSet *status_set = userdata;
const ExitStatusSet *status_set = userdata;
unsigned n;
Iterator i;
void *id;
int r;
assert(bus);
@@ -56,13 +56,10 @@ static int property_get_exit_status_set(
if (r < 0)
return r;
SET_FOREACH(id, status_set->status, i) {
int32_t val = PTR_TO_INT(id);
BITMAP_FOREACH(n, &status_set->status, i) {
assert(n < 256);
if (val < 0 || val > 255)
continue;
r = sd_bus_message_append_basic(reply, 'i', &val);
r = sd_bus_message_append_basic(reply, 'i', &n);
if (r < 0)
return r;
}
@@ -75,15 +72,14 @@ static int property_get_exit_status_set(
if (r < 0)
return r;
SET_FOREACH(id, status_set->signal, i) {
int32_t val = PTR_TO_INT(id);
BITMAP_FOREACH(n, &status_set->signal, i) {
const char *str;
str = signal_to_string((int) val);
str = signal_to_string(n);
if (!str)
continue;
r = sd_bus_message_append_basic(reply, 'i', &val);
r = sd_bus_message_append_basic(reply, 'i', &n);
if (r < 0)
return r;
}
@@ -196,11 +192,7 @@ static int bus_set_transient_exit_status(
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid status code in %s: %"PRIi32, name, status[i]);
if (!UNIT_WRITE_FLAGS_NOOP(flags)) {
r = set_ensure_allocated(&status_set->status, NULL);
if (r < 0)
return r;
r = set_put(status_set->status, INT_TO_PTR((int) status[i]));
r = bitmap_set(&status_set->status, status[i]);
if (r < 0)
return r;
@@ -216,11 +208,7 @@ static int bus_set_transient_exit_status(
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid signal in %s: %"PRIi32, name, signal[i]);
if (!UNIT_WRITE_FLAGS_NOOP(flags)) {
r = set_ensure_allocated(&status_set->signal, NULL);
if (r < 0)
return r;
r = set_put(status_set->signal, INT_TO_PTR((int) signal[i]));
r = bitmap_set(&status_set->signal, signal[i]);
if (r < 0)
return r;
+4 -8
View File
@@ -3937,7 +3937,7 @@ int config_parse_set_status(
FOREACH_WORD(word, l, rvalue, state) {
_cleanup_free_ char *temp;
int val;
Set **set;
Bitmap *bitmap;
temp = strndup(word, l);
if (!temp)
@@ -3951,20 +3951,16 @@ int config_parse_set_status(
log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse value, ignoring: %s", word);
continue;
}
set = &status_set->signal;
bitmap = &status_set->signal;
} else {
if (val < 0 || val > 255) {
log_syntax(unit, LOG_ERR, filename, line, 0, "Value %d is outside range 0-255, ignoring", val);
continue;
}
set = &status_set->status;
bitmap = &status_set->status;
}
r = set_ensure_allocated(set, NULL);
if (r < 0)
return log_oom();
r = set_put(*set, INT_TO_PTR(val));
r = bitmap_set(bitmap, val);
if (r < 0)
return log_oom();
}
-6
View File
@@ -12,12 +12,6 @@
#include "macro.h"
#include "memory-util.h"
struct Bitmap {
uint64_t *bitmaps;
size_t n_bitmaps;
size_t bitmaps_allocated;
};
/* Bitmaps are only meant to store relatively small numbers
* (corresponding to, say, an enum), so it is ok to limit
* the max entry. 64k should be plenty. */
+5 -1
View File
@@ -6,7 +6,11 @@
#include "hashmap.h"
#include "macro.h"
typedef struct Bitmap Bitmap;
typedef struct Bitmap {
uint64_t *bitmaps;
size_t n_bitmaps;
size_t bitmaps_allocated;
} Bitmap;
Bitmap *bitmap_new(void);
Bitmap *bitmap_copy(Bitmap *b);
+12 -15
View File
@@ -134,18 +134,19 @@ int exit_status_from_string(const char *s) {
return val;
}
bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success_status) {
bool is_clean_exit(int code, int status, ExitClean clean, const ExitStatusSet *success_status) {
if (code == CLD_EXITED)
return status == 0 ||
(success_status &&
set_contains(success_status->status, INT_TO_PTR(status)));
bitmap_isset(&success_status->status, status));
/* If a daemon does not implement handlers for some of the signals that's not considered an unclean shutdown */
/* If a daemon does not implement handlers for some of the signals, we do not consider this an
unclean shutdown */
if (code == CLD_KILLED)
return
(clean == EXIT_CLEAN_DAEMON && IN_SET(status, SIGHUP, SIGINT, SIGTERM, SIGPIPE)) ||
(success_status &&
set_contains(success_status->signal, INT_TO_PTR(status)));
bitmap_isset(&success_status->signal, status));
return false;
}
@@ -153,26 +154,22 @@ bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success
void exit_status_set_free(ExitStatusSet *x) {
assert(x);
x->status = set_free(x->status);
x->signal = set_free(x->signal);
bitmap_clear(&x->status);
bitmap_clear(&x->signal);
}
bool exit_status_set_is_empty(ExitStatusSet *x) {
bool exit_status_set_is_empty(const ExitStatusSet *x) {
if (!x)
return true;
return set_isempty(x->status) && set_isempty(x->signal);
return bitmap_isclear(&x->status) && bitmap_isclear(&x->signal);
}
bool exit_status_set_test(ExitStatusSet *x, int code, int status) {
if (exit_status_set_is_empty(x))
return false;
if (code == CLD_EXITED && set_contains(x->status, INT_TO_PTR(status)))
bool exit_status_set_test(const ExitStatusSet *x, int code, int status) {
if (code == CLD_EXITED && bitmap_isset(&x->status, status))
return true;
if (IN_SET(code, CLD_KILLED, CLD_DUMPED) && set_contains(x->signal, INT_TO_PTR(status)))
if (IN_SET(code, CLD_KILLED, CLD_DUMPED) && bitmap_isset(&x->signal, status))
return true;
return false;
+6 -6
View File
@@ -3,9 +3,9 @@
#include <stdbool.h>
#include "bitmap.h"
#include "hashmap.h"
#include "macro.h"
#include "set.h"
/* This defines pretty names for the LSB 'start' verb exit codes. Note that they shouldn't be confused with
* the LSB 'status' verb exit codes which are defined very differently. For details see:
@@ -83,8 +83,8 @@ typedef enum ExitStatusClass {
} ExitStatusClass;
typedef struct ExitStatusSet {
Set *status;
Set *signal;
Bitmap status;
Bitmap signal;
} ExitStatusSet;
const char* exit_status_to_string(int code, ExitStatusClass class) _const_;
@@ -103,8 +103,8 @@ typedef enum ExitClean {
EXIT_CLEAN_COMMAND,
} ExitClean;
bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success_status);
bool is_clean_exit(int code, int status, ExitClean clean, const ExitStatusSet *success_status);
void exit_status_set_free(ExitStatusSet *x);
bool exit_status_set_is_empty(ExitStatusSet *x);
bool exit_status_set_test(ExitStatusSet *x, int code, int status);
bool exit_status_set_is_empty(const ExitStatusSet *x);
bool exit_status_set_test(const ExitStatusSet *x, int code, int status);
@@ -39,6 +39,7 @@
#include "plymouth-util.h"
#include "pretty-print.h"
#include "process-util.h"
#include "set.h"
#include "signal-util.h"
#include "socket-util.h"
#include "string-util.h"