From b42549ad69ab51789d5b8fa9f1dfc6c2fe45f43e Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Wed, 12 Jul 2017 22:08:58 -0700 Subject: [PATCH 1/2] journal: return mapped size from mmap_cache_get() If requested, return the actual mapping size to the caller in addition to the address. journal_file_move_to_object() often performs two successive mmap_cache_get() calls via journal_file_move_to(); one to get the object header, then another to get the entire object when it's larger than the header's size. If mmap_cache_get() returned the actual mapping's size, it's probable that the second mmap_cache_get() could be skipped when the established mapping already encompassed the desired size. --- src/journal/journal-file.c | 8 ++++---- src/journal/journal-verify.c | 2 +- src/journal/mmap-cache.c | 27 ++++++++++++++++++++------- src/journal/mmap-cache.h | 3 ++- src/journal/test-mmap-cache.c | 10 +++++----- 5 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 4ff38de2e6..05cbf08a35 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -749,7 +749,7 @@ static int journal_file_move_to(JournalFile *f, ObjectType type, bool keep_alway return -EADDRNOTAVAIL; } - return mmap_cache_get(f->mmap, f->cache_fd, f->prot, type_to_context(type), keep_always, offset, size, &f->last_stat, ret); + return mmap_cache_get(f->mmap, f->cache_fd, f->prot, type_to_context(type), keep_always, offset, size, &f->last_stat, ret, NULL); } static uint64_t minimum_header_size(Object *o) { @@ -991,7 +991,7 @@ int journal_file_map_data_hash_table(JournalFile *f) { OBJECT_DATA_HASH_TABLE, true, p, s, - &t); + &t, NULL); if (r < 0) return r; @@ -1017,7 +1017,7 @@ int journal_file_map_field_hash_table(JournalFile *f) { OBJECT_FIELD_HASH_TABLE, true, p, s, - &t); + &t, NULL); if (r < 0) return r; @@ -3196,7 +3196,7 @@ int journal_file_open( goto fail; } - r = mmap_cache_get(f->mmap, f->cache_fd, f->prot, CONTEXT_HEADER, true, 0, PAGE_ALIGN(sizeof(Header)), &f->last_stat, &h); + r = mmap_cache_get(f->mmap, f->cache_fd, f->prot, CONTEXT_HEADER, true, 0, PAGE_ALIGN(sizeof(Header)), &f->last_stat, &h, NULL); if (r < 0) goto fail; diff --git a/src/journal/journal-verify.c b/src/journal/journal-verify.c index 9feb5b5ae6..3f4c38ccde 100644 --- a/src/journal/journal-verify.c +++ b/src/journal/journal-verify.c @@ -392,7 +392,7 @@ static int contains_uint64(MMapCache *m, MMapFileDescriptor *f, uint64_t n, uint c = (a + b) / 2; - r = mmap_cache_get(m, f, PROT_READ|PROT_WRITE, 0, false, c * sizeof(uint64_t), sizeof(uint64_t), NULL, (void **) &z); + r = mmap_cache_get(m, f, PROT_READ|PROT_WRITE, 0, false, c * sizeof(uint64_t), sizeof(uint64_t), NULL, (void **) &z, NULL); if (r < 0) return r; diff --git a/src/journal/mmap-cache.c b/src/journal/mmap-cache.c index 5dfda73c56..546710cdc9 100644 --- a/src/journal/mmap-cache.c +++ b/src/journal/mmap-cache.c @@ -338,7 +338,8 @@ static int try_context( bool keep_always, uint64_t offset, size_t size, - void **ret) { + void **ret, + size_t *ret_size) { Context *c; @@ -370,6 +371,9 @@ static int try_context( c->window->keep_always = c->window->keep_always || keep_always; *ret = (uint8_t*) c->window->ptr + (offset - c->window->offset); + if (ret_size) + *ret_size = c->window->size - (offset - c->window->offset); + return 1; } @@ -381,7 +385,8 @@ static int find_mmap( bool keep_always, uint64_t offset, size_t size, - void **ret) { + void **ret, + size_t *ret_size) { Window *w; Context *c; @@ -409,6 +414,9 @@ static int find_mmap( w->keep_always = w->keep_always || keep_always; *ret = (uint8_t*) w->ptr + (offset - w->offset); + if (ret_size) + *ret_size = w->size - (offset - w->offset); + return 1; } @@ -448,7 +456,8 @@ static int add_mmap( uint64_t offset, size_t size, struct stat *st, - void **ret) { + void **ret, + size_t *ret_size) { uint64_t woffset, wsize; Context *c; @@ -508,6 +517,9 @@ static int add_mmap( LIST_PREPEND(by_window, w->contexts, c); *ret = (uint8_t*) w->ptr + (offset - w->offset); + if (ret_size) + *ret_size = w->size - (offset - w->offset); + return 1; outofmem: @@ -524,7 +536,8 @@ int mmap_cache_get( uint64_t offset, size_t size, struct stat *st, - void **ret) { + void **ret, + size_t *ret_size) { int r; @@ -536,14 +549,14 @@ int mmap_cache_get( assert(context < MMAP_CACHE_MAX_CONTEXTS); /* Check whether the current context is the right one already */ - r = try_context(m, f, prot, context, keep_always, offset, size, ret); + r = try_context(m, f, prot, context, keep_always, offset, size, ret, ret_size); if (r != 0) { m->n_hit++; return r; } /* Search for a matching mmap */ - r = find_mmap(m, f, prot, context, keep_always, offset, size, ret); + r = find_mmap(m, f, prot, context, keep_always, offset, size, ret, ret_size); if (r != 0) { m->n_hit++; return r; @@ -552,7 +565,7 @@ int mmap_cache_get( m->n_missed++; /* Create a new mmap */ - return add_mmap(m, f, prot, context, keep_always, offset, size, st, ret); + return add_mmap(m, f, prot, context, keep_always, offset, size, st, ret, ret_size); } unsigned mmap_cache_get_hit(MMapCache *m) { diff --git a/src/journal/mmap-cache.h b/src/journal/mmap-cache.h index 7b33218563..cf6af1906a 100644 --- a/src/journal/mmap-cache.h +++ b/src/journal/mmap-cache.h @@ -41,7 +41,8 @@ int mmap_cache_get( uint64_t offset, size_t size, struct stat *st, - void **ret); + void **ret, + size_t *ret_size); MMapFileDescriptor * mmap_cache_add_fd(MMapCache *m, int fd); void mmap_cache_free_fd(MMapCache *m, MMapFileDescriptor *f); diff --git a/src/journal/test-mmap-cache.c b/src/journal/test-mmap-cache.c index c51b069f8b..702434efd2 100644 --- a/src/journal/test-mmap-cache.c +++ b/src/journal/test-mmap-cache.c @@ -51,23 +51,23 @@ int main(int argc, char *argv[]) { assert_se(z >= 0); unlink(pz); - r = mmap_cache_get(m, fx, PROT_READ, 0, false, 1, 2, NULL, &p); + r = mmap_cache_get(m, fx, PROT_READ, 0, false, 1, 2, NULL, &p, NULL); assert_se(r >= 0); - r = mmap_cache_get(m, fx, PROT_READ, 0, false, 2, 2, NULL, &q); + r = mmap_cache_get(m, fx, PROT_READ, 0, false, 2, 2, NULL, &q, NULL); assert_se(r >= 0); assert_se((uint8_t*) p + 1 == (uint8_t*) q); - r = mmap_cache_get(m, fx, PROT_READ, 1, false, 3, 2, NULL, &q); + r = mmap_cache_get(m, fx, PROT_READ, 1, false, 3, 2, NULL, &q, NULL); assert_se(r >= 0); assert_se((uint8_t*) p + 2 == (uint8_t*) q); - r = mmap_cache_get(m, fx, PROT_READ, 0, false, 16ULL*1024ULL*1024ULL, 2, NULL, &p); + r = mmap_cache_get(m, fx, PROT_READ, 0, false, 16ULL*1024ULL*1024ULL, 2, NULL, &p, NULL); assert_se(r >= 0); - r = mmap_cache_get(m, fx, PROT_READ, 1, false, 16ULL*1024ULL*1024ULL+1, 2, NULL, &q); + r = mmap_cache_get(m, fx, PROT_READ, 1, false, 16ULL*1024ULL*1024ULL+1, 2, NULL, &q, NULL); assert_se(r >= 0); assert_se((uint8_t*) p + 1 == (uint8_t*) q); From b439282e0b496923cb144954cd479949fff3aa01 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Wed, 12 Jul 2017 22:17:06 -0700 Subject: [PATCH 2/2] journal: avoid unnecessary mmap_cache_get() calls journal_file_move_to_object() can skip the second journal_file_move_to() call if the first one already mapped a sufficiently large area. Now that mmap_cache_get() returns the size of the mapped area when asked, ask for the size and only perform the second call if the required size exceeds the mapped size instead of the object header size. This results in a nice performance boost in my testing, even with a corpus of many small logs burning much CPU time elsewhere: Before: # time ./journalctl -b -1 --no-pager > /dev/null real 0m16.330s user 0m16.281s sys 0m0.046s # time ./journalctl -b -1 --no-pager > /dev/null real 0m16.409s user 0m16.358s sys 0m0.048s # time ./journalctl -b -1 --no-pager > /dev/null real 0m16.625s user 0m16.558s sys 0m0.061s After: # time ./journalctl -b -1 --no-pager > /dev/null real 0m15.311s user 0m15.257s sys 0m0.046s # time ./journalctl -b -1 --no-pager > /dev/null real 0m15.201s user 0m15.135s sys 0m0.062s # time ./journalctl -b -1 --no-pager > /dev/null real 0m15.170s user 0m15.113s sys 0m0.053s --- src/journal/journal-file.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 05cbf08a35..84e64aef6c 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -727,7 +727,7 @@ static unsigned type_to_context(ObjectType type) { return type > OBJECT_UNUSED && type < _OBJECT_TYPE_MAX ? type : 0; } -static int journal_file_move_to(JournalFile *f, ObjectType type, bool keep_always, uint64_t offset, uint64_t size, void **ret) { +static int journal_file_move_to(JournalFile *f, ObjectType type, bool keep_always, uint64_t offset, uint64_t size, void **ret, size_t *ret_size) { int r; assert(f); @@ -749,7 +749,7 @@ static int journal_file_move_to(JournalFile *f, ObjectType type, bool keep_alway return -EADDRNOTAVAIL; } - return mmap_cache_get(f->mmap, f->cache_fd, f->prot, type_to_context(type), keep_always, offset, size, &f->last_stat, ret, NULL); + return mmap_cache_get(f->mmap, f->cache_fd, f->prot, type_to_context(type), keep_always, offset, size, &f->last_stat, ret, ret_size); } static uint64_t minimum_header_size(Object *o) { @@ -773,6 +773,7 @@ static uint64_t minimum_header_size(Object *o) { int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset, Object **ret) { int r; void *t; + size_t tsize; Object *o; uint64_t s; @@ -791,7 +792,7 @@ int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset return -EBADMSG; } - r = journal_file_move_to(f, type, false, offset, sizeof(ObjectHeader), &t); + r = journal_file_move_to(f, type, false, offset, sizeof(ObjectHeader), &t, &tsize); if (r < 0) return r; @@ -822,8 +823,8 @@ int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset return -EBADMSG; } - if (s > sizeof(ObjectHeader)) { - r = journal_file_move_to(f, type, false, offset, s, &t); + if (s > tsize) { + r = journal_file_move_to(f, type, false, offset, s, &t, NULL); if (r < 0) return r; @@ -893,7 +894,7 @@ int journal_file_append_object(JournalFile *f, ObjectType type, uint64_t size, O if (r < 0) return r; - r = journal_file_move_to(f, type, false, p, size, &t); + r = journal_file_move_to(f, type, false, p, size, &t, NULL); if (r < 0) return r;