[systemd-devel] [RFC/PATCH] journalctl: Improve boot ID lookup

Jan Janssen medhefgo at web.de
Wed Apr 1 09:41:42 PDT 2015


This method shouldn't provide any noticeable speedup for the --list-boots
case, but any offset based lookup should be greatly improved.
We now don't have to aggregate the full boot listing just so we can jump
to specific position, which can be a real pain on big journals just for
a mere "-b -1" case.

--list-boots might get a little slower, but not by much. And keeping
--boot and --list-boots' idea of boots consistent should justify the slight
increase.

Note that there can be a change in boot order in this --list-boots version
because it will use the order of boots in journals, not the realtime stamp
stored in them. That's arguably better, though.

https://bugs.freedesktop.org/show_bug.cgi?id=72601
---
Hi,

I can't believe I didn't come up with this one sooner. The details how it works
are in the comments, but I could use some testing by people who have tons of
boots in their journal. I only have 58, which doesn't make --boot -1 that big a
pain, but I still do get an improvement: ~2s without and ~0s lookup with this patch
applied (using /proc/sys/vm/drop_caches of course).

The patch could use some testing/timing with huge journals, and *especially* with
some corrupted journals in the mix, since I have none right now (fresh computer, yay).

Jan

 src/journal/journalctl.c | 301 +++++++++++++++++++++++++++++++----------------
 1 file changed, 200 insertions(+), 101 deletions(-)

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index b4f88bc..bdfa0b0 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -128,6 +128,7 @@ typedef struct boot_id_t {
         sd_id128_t id;
         uint64_t first;
         uint64_t last;
+        LIST_FIELDS(struct boot_id_t, boot_list);
 } boot_id_t;
 
 static void pager_open_if_enabled(void) {
@@ -851,111 +852,234 @@ static int add_matches(sd_journal *j, char **args) {
         return 0;
 }
 
-static int boot_id_cmp(const void *a, const void *b) {
-        uint64_t _a, _b;
+static int discover_next_boot(sd_journal *j,
+                              boot_id_t **boot,
+                              char **cursor,
+                              bool advance_left,
+                              bool read_realtime) {
+        int r;
+        char match[9+32+1] = "_BOOT_ID=";
+        _cleanup_free_ boot_id_t *next_boot = NULL;
 
-        _a = ((const boot_id_t *)a)->first;
-        _b = ((const boot_id_t *)b)->first;
+        assert(j);
+        assert(boot);
+        assert(cursor);
 
-        return _a < _b ? -1 : (_a > _b ? 1 : 0);
-}
+        /* We expect the cursor to point us to the last position
+         * of a boot, so that the next invocation of sd_j_next would be
+         * from a different boot. We collect any information we desire
+         * and then jump to the last location of the new boot by using
+         * a _BOOT_ID match and coming from the other journal direction
+         * (the tail). Since we wouldn't then be able to advance to the
+         * next boot using sd_j_next, we take a cursor and rinse and repeat. */
 
-static int get_boots(sd_journal *j,
-                     boot_id_t **boots,
-                     unsigned int *count,
-                     boot_id_t *query_ref_boot) {
-        int r;
-        const void *data;
-        size_t length, allocated = 0;
+        sd_journal_flush_matches(j);
 
-        assert(j);
-        assert(boots);
-        assert(count);
+        if (*cursor) {
+                r = sd_journal_seek_cursor(j, *cursor);
+                if (r < 0)
+                        return r;
+
+                if (advance_left)
+                        r = sd_journal_previous(j);
+                else
+                        r = sd_journal_next(j);
+                if (r < 0)
+                        return r;
+                else if (r == 0)
+                        return -ENODATA; /* We were here last time, odd. */
+        } else {
+                if (advance_left)
+                        r = sd_journal_seek_tail(j);
+                else
+                        r = sd_journal_seek_head(j);
+                if (r < 0)
+                        return r;
+        }
 
-        r = sd_journal_query_unique(j, "_BOOT_ID");
+        /* Advance to next boot. */
+        if (advance_left)
+                r = sd_journal_previous(j);
+        else
+                r = sd_journal_next(j);
         if (r < 0)
                 return r;
+        else if (r == 0) {
+                /* End of journal, yay. */
+                *boot = NULL;
+                return 0;
+        }
 
-        *count = 0;
-        SD_JOURNAL_FOREACH_UNIQUE(j, data, length) {
-                boot_id_t *id;
+        next_boot = new0(boot_id_t, 1);
+        if (!next_boot)
+                return log_oom();
 
-                assert(startswith(data, "_BOOT_ID="));
+        r = sd_journal_get_monotonic_usec(j, NULL, &next_boot->id);
+        if (r < 0)
+                return r;
 
-                if (!GREEDY_REALLOC(*boots, allocated, *count + 1))
-                        return log_oom();
+        if (read_realtime) {
+                r = sd_journal_get_realtime_usec(j, &next_boot->first);
+                if (r < 0)
+                        return r;
+        }
+
+        /* Now seek to the last occurrence of this boot ID and save
+         * the position in the cursor for next time. */
+        sd_id128_to_string(next_boot->id, match + 9);
+        r = sd_journal_add_match(j, match, sizeof(match) - 1);
+        if (r < 0)
+                return r;
 
-                id = *boots + *count;
+        if (advance_left)
+                r = sd_journal_seek_head(j);
+        else
+                r = sd_journal_seek_tail(j);
+        if (r < 0)
+                return r;
 
-                r = sd_id128_from_string(((const char *)data) + strlen("_BOOT_ID="), &id->id);
+        if (advance_left)
+                r = sd_journal_next(j);
+        else
+                r = sd_journal_previous(j);
+        if (r < 0)
+                return r;
+        else if (r == 0)
+                return -ENODATA; /* This shouldn't happen. We just came from this very boot ID. */
+
+        if (read_realtime) {
+                r = sd_journal_get_realtime_usec(j, &next_boot->last);
                 if (r < 0)
-                        continue;
+                        return r;
+        }
+
+        free(*cursor);
+        *cursor = NULL;
+        r = sd_journal_get_cursor(j, cursor);
+        if (r < 0)
+                return r;
+
+        *boot = next_boot;
+        next_boot = NULL;
+        return 0;
+}
+
+static int get_boots(sd_journal *j,
+                     boot_id_t **boots,
+                     boot_id_t *query_ref_boot,
+                     int ref_boot_offset) {
+        bool skip_once;
+        int r, count = 0;
+        boot_id_t *head = NULL, *tail = NULL;
+        _cleanup_free_ char *cursor = NULL;
+        const bool advance_left = query_ref_boot && ref_boot_offset <= 0;
+
+        assert(j);
 
-                r = sd_journal_add_match(j, data, length);
+        /* Adjust for the fact that offset 0 is
+         * the last (read current) boot. */
+        skip_once = query_ref_boot && sd_id128_is_null(query_ref_boot->id) && ref_boot_offset < 0;
+
+        if (query_ref_boot && !sd_id128_is_null(query_ref_boot->id)) {
+                char match[9+32+1] = "_BOOT_ID=";
+
+                /* Get a cursor for the boot ID lookup. Take into
+                 * account that it needs to be the earliest (offset <= 0)
+                 * or latest entry (offset > 0) for that specific
+                 * boot ID.
+                 * Note that discover_next_boot() will handle the discovery
+                 * if we don't have a reference boot ID. */
+
+                sd_journal_flush_matches(j);
+
+                sd_id128_to_string(query_ref_boot->id, match + 9);
+                r = sd_journal_add_match(j, match, sizeof(match) - 1);
                 if (r < 0)
                         return r;
 
-                r = sd_journal_seek_head(j);
+                if (advance_left)
+                        r = sd_journal_seek_head(j);
+                else
+                        r = sd_journal_seek_tail(j);
                 if (r < 0)
                         return r;
 
-                r = sd_journal_next(j);
+                if (advance_left)
+                        r = sd_journal_next(j);
+                else
+                        r = sd_journal_previous(j);
                 if (r < 0)
                         return r;
                 else if (r == 0)
-                        goto flush;
+                        goto finish;
+                else if (ref_boot_offset == 0) {
+                        count = 1;
+                        goto finish;
+                }
 
-                r = sd_journal_get_realtime_usec(j, &id->first);
+                r = sd_journal_get_cursor(j, &cursor);
                 if (r < 0)
                         return r;
+        }
 
-                if (query_ref_boot) {
-                        id->last = 0;
-                        if (sd_id128_equal(id->id, query_ref_boot->id))
-                                *query_ref_boot = *id;
-                } else {
-                        r = sd_journal_seek_tail(j);
-                        if (r < 0)
-                                return r;
-
-                        r = sd_journal_previous(j);
-                        if (r < 0)
-                                return r;
-                        else if (r == 0)
-                                goto flush;
+        while (true) {
+                _cleanup_free_ boot_id_t *current = NULL;
 
-                        r = sd_journal_get_realtime_usec(j, &id->last);
-                        if (r < 0)
-                                return r;
+                r = discover_next_boot(j, &current, &cursor, advance_left, !query_ref_boot);
+                if (r < 0) {
+                        boot_id_t *id, *id_next;
+                        LIST_FOREACH_SAFE(boot_list, id, id_next, head)
+                                free(id);
+                        return r;
                 }
 
-                (*count)++;
-        flush:
-                sd_journal_flush_matches(j);
+                if (!current)
+                        break;
+
+                if (query_ref_boot) {
+                        if (!skip_once)
+                                ref_boot_offset += advance_left ? 1 : -1;
+                        skip_once = false;
+
+                        if (ref_boot_offset == 0) {
+                                count = 1;
+                                query_ref_boot->id = current->id;
+                                break;
+                        }
+                } else {
+                        LIST_INSERT_AFTER(boot_list, head, tail, current);
+                        tail = current;
+                        current = NULL;
+                        count++;
+                }
         }
 
-        qsort_safe(*boots, *count, sizeof(boot_id_t), boot_id_cmp);
-        return 0;
+finish:
+        if (boots)
+                *boots = head;
+
+        sd_journal_flush_matches(j);
+
+        return count;
 }
 
 static int list_boots(sd_journal *j) {
-        int r, w, i;
-        unsigned int count;
-        boot_id_t *id;
-        _cleanup_free_ boot_id_t *all_ids = NULL;
+        int w, i, count;
+        boot_id_t *id, *id_next, *all_ids;
 
         assert(j);
 
-        r = get_boots(j, &all_ids, &count, NULL);
-        if (r < 0)
-                return r;
+        count = get_boots(j, &all_ids, NULL, 0);
+        if (count <= 0)
+                return count;
 
         pager_open_if_enabled();
 
         /* numbers are one less, but we need an extra char for the sign */
         w = DECIMAL_STR_WIDTH(count - 1) + 1;
 
-        for (id = all_ids, i = 0; id < all_ids + count; id++, i++) {
+        i = 0;
+        LIST_FOREACH_SAFE(boot_list, id, id_next, all_ids) {
                 char a[FORMAT_TIMESTAMP_MAX], b[FORMAT_TIMESTAMP_MAX];
 
                 printf("% *i " SD_ID128_FORMAT_STR " %s—%s\n",
@@ -963,39 +1087,8 @@ static int list_boots(sd_journal *j) {
                        SD_ID128_FORMAT_VAL(id->id),
                        format_timestamp_maybe_utc(a, sizeof(a), id->first),
                        format_timestamp_maybe_utc(b, sizeof(b), id->last));
-        }
-
-        return 0;
-}
-
-static int get_boot_id_by_offset(sd_journal *j, sd_id128_t *boot_id, int offset) {
-        int r;
-        unsigned int count;
-        boot_id_t ref_boot_id = {}, *id;
-        _cleanup_free_ boot_id_t *all_ids = NULL;
-
-        assert(j);
-        assert(boot_id);
-
-        ref_boot_id.id = *boot_id;
-        r = get_boots(j, &all_ids, &count, &ref_boot_id);
-        if (r < 0)
-                return r;
-
-        if (sd_id128_equal(*boot_id, SD_ID128_NULL)) {
-                if (offset > (int) count || offset <= -(int)count)
-                        return -EADDRNOTAVAIL;
-
-                *boot_id = all_ids[(offset <= 0)*count + offset - 1].id;
-        } else {
-                id = bsearch(&ref_boot_id, all_ids, count, sizeof(boot_id_t), boot_id_cmp);
-
-                if (!id ||
-                    offset <= 0 ? (id - all_ids) + offset < 0 :
-                                    (id - all_ids) + offset >= (int) count)
-                        return -EADDRNOTAVAIL;
-
-                *boot_id = (id + offset)->id;
+                i++;
+                free(id);
         }
 
         return 0;
@@ -1004,6 +1097,7 @@ static int get_boot_id_by_offset(sd_journal *j, sd_id128_t *boot_id, int offset)
 static int add_boot(sd_journal *j) {
         char match[9+32+1] = "_BOOT_ID=";
         int r;
+        boot_id_t ref_boot_id = {};
 
         assert(j);
 
@@ -1013,17 +1107,22 @@ static int add_boot(sd_journal *j) {
         if (arg_boot_offset == 0 && sd_id128_equal(arg_boot_id, SD_ID128_NULL))
                 return add_match_this_boot(j, arg_machine);
 
-        r = get_boot_id_by_offset(j, &arg_boot_id, arg_boot_offset);
-        if (r < 0) {
-                if (sd_id128_equal(arg_boot_id, SD_ID128_NULL))
-                        log_error_errno(r, "Failed to look up boot %+i: %m", arg_boot_offset);
+        ref_boot_id.id = arg_boot_id;
+        r = get_boots(j, NULL, &ref_boot_id, arg_boot_offset);
+        assert(r <= 1);
+        if (r <= 0) {
+                const char *reason = (r == 0) ? "No such boot ID in journal" : strerror(-r);
+
+                if (sd_id128_is_null(arg_boot_id))
+                        log_error("Failed to look up boot %+i: %s", arg_boot_offset, reason);
                 else
                         log_error("Failed to look up boot ID "SD_ID128_FORMAT_STR"%+i: %s",
-                                  SD_ID128_FORMAT_VAL(arg_boot_id), arg_boot_offset, strerror(-r));
-                return r;
+                                  SD_ID128_FORMAT_VAL(arg_boot_id), arg_boot_offset, reason);
+
+                return r == 0 ? -ENODATA : r;
         }
 
-        sd_id128_to_string(arg_boot_id, match + 9);
+        sd_id128_to_string(ref_boot_id.id, match + 9);
 
         r = sd_journal_add_match(j, match, sizeof(match) - 1);
         if (r < 0)
-- 
2.3.5



More information about the systemd-devel mailing list