From f02344c0a4daf71dfe64d6c766b089092b43caed Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Sat, 10 Jun 2023 19:52:05 +0300 Subject: [PATCH] ...and partially remove the perversion with bitmap inlining --- src/blockstore_flush.cpp | 8 +++----- src/blockstore_impl.cpp | 1 + src/blockstore_impl.h | 1 + src/blockstore_init.cpp | 14 ++------------ src/blockstore_read.cpp | 8 +++----- src/blockstore_rollback.cpp | 5 +---- src/blockstore_sync.cpp | 2 +- src/blockstore_write.cpp | 12 ++++++------ 8 files changed, 18 insertions(+), 33 deletions(-) diff --git a/src/blockstore_flush.cpp b/src/blockstore_flush.cpp index fc60cb92..2a27da81 100644 --- a/src/blockstore_flush.cpp +++ b/src/blockstore_flush.cpp @@ -641,8 +641,7 @@ void journal_flusher_co::update_metadata_entry() } // Copy latest external bitmap/attributes { - uint64_t dyn_size = bs->dsk.dirty_dyn_size(dirty_end->second.offset, dirty_end->second.len); - void *dyn_ptr = dyn_size > sizeof(void*) + void *dyn_ptr = bs->alloc_dyn_data ? (uint8_t*)dirty_end->second.dyn_data+sizeof(int) : (uint8_t*)&dirty_end->second.dyn_data; memcpy(new_clean_bitmap + bs->dsk.clean_entry_bitmap_size, dyn_ptr, bs->dsk.clean_entry_bitmap_size); } @@ -955,8 +954,7 @@ void journal_flusher_co::scan_dirty() { // FIXME Remove this > sizeof(void*) inline perversion from everywhere. // I think it doesn't matter but I couldn't stop myself from implementing it :) - uint64_t dyn_size = bs->dsk.dirty_dyn_size(dirty_it->second.offset, dirty_it->second.len); - uint8_t* dyn_from = (uint8_t*)(dyn_size > sizeof(void*) + uint8_t* dyn_from = (uint8_t*)(bs->alloc_dyn_data ? (uint8_t*)dirty_it->second.dyn_data+sizeof(int) : (uint8_t*)&dirty_it->second.dyn_data) + bs->dsk.clean_entry_bitmap_size; it->csum_buf = dyn_from + (it->offset/bs->dsk.csum_block_size - @@ -986,7 +984,7 @@ void journal_flusher_co::scan_dirty() clean_init_bitmap = true; clean_bitmap_offset = dirty_it->second.offset; clean_bitmap_len = dirty_it->second.len; - clean_init_dyn_ptr = bs->dsk.dirty_dyn_size(clean_bitmap_offset, clean_bitmap_len) > sizeof(void*) + clean_init_dyn_ptr = bs->alloc_dyn_data ? (uint8_t*)dirty_it->second.dyn_data+sizeof(int) : (uint8_t*)&dirty_it->second.dyn_data; skip_copy = true; } diff --git a/src/blockstore_impl.cpp b/src/blockstore_impl.cpp index f98b93cc..36da71af 100644 --- a/src/blockstore_impl.cpp +++ b/src/blockstore_impl.cpp @@ -13,6 +13,7 @@ blockstore_impl_t::blockstore_impl_t(blockstore_config_t & config, ring_loop_t * initialized = 0; parse_config(config, true); zero_object = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, dsk.data_block_size); + alloc_dyn_data = dsk.clean_dyn_size > sizeof(void*) || dsk.csum_block_size > 0; try { dsk.open_data(); diff --git a/src/blockstore_impl.h b/src/blockstore_impl.h index b56a4ea0..e50a9cc5 100644 --- a/src/blockstore_impl.h +++ b/src/blockstore_impl.h @@ -283,6 +283,7 @@ class blockstore_impl_t journal_flusher_t *flusher; int big_to_flush = 0; int write_iodepth = 0; + bool alloc_dyn_data = false; // clean data blocks referenced by read operations std::map used_clean_objects; diff --git a/src/blockstore_init.cpp b/src/blockstore_init.cpp index 7cfea298..6680210c 100644 --- a/src/blockstore_init.cpp +++ b/src/blockstore_init.cpp @@ -891,17 +891,12 @@ int blockstore_init_journal::handle_journal_part(void *buf, uint64_t done_pos, u uint64_t dyn_size = bs->dsk.dirty_dyn_size(je->small_write.offset, je->small_write.len); void *dyn = NULL; void *dyn_from = (uint8_t*)je + sizeof(journal_entry_small_write); - if (dyn_size <= sizeof(void*)) + if (!bs->alloc_dyn_data) { // Bitmap without checksum is only 4 bytes for 128k objects, save it inline // It can even contain 4 byte bitmap + 4 byte CRC32 for 4 kb writes :) memcpy(&dyn, dyn_from, dyn_size); } - else if (bs->journal.inmemory) - { - // Journal is kept in memory, refer to it instead of allocating a buffer - dyn = dyn_from; - } else { // FIXME Using large blockstore objects will result in a lot of small @@ -981,16 +976,11 @@ int blockstore_init_journal::handle_journal_part(void *buf, uint64_t done_pos, u uint64_t dyn_size = bs->dsk.dirty_dyn_size(je->big_write.offset, je->big_write.len); void *dyn = NULL; void *dyn_from = (uint8_t*)je + sizeof(journal_entry_big_write); - if (dyn_size <= sizeof(void*)) + if (!bs->alloc_dyn_data) { // Bitmap without checksum is only 4 bytes for 128k objects, save it inline memcpy(&dyn, dyn_from, dyn_size); } - else if (bs->journal.inmemory) - { - // Journal is kept in memory, refer to it instead of allocating a buffer - dyn = dyn_from; - } else { // FIXME Using large blockstore objects will result in a lot of small diff --git a/src/blockstore_read.cpp b/src/blockstore_read.cpp index 1c3927b1..9b363672 100644 --- a/src/blockstore_read.cpp +++ b/src/blockstore_read.cpp @@ -412,9 +412,8 @@ int blockstore_impl_t::dequeue_read(blockstore_op_t *read_op) FINISH_OP(read_op); return 2; } - size_t dyn_size = dsk.dirty_dyn_size(dirty.offset, dirty.len); - int *dyn_data = (int*)(dsk.csum_block_size > 0 && dyn_size > sizeof(void*) ? dirty.dyn_data : NULL); - uint8_t *bmp_ptr = (dyn_size > sizeof(void*) + int *dyn_data = (int*)(dsk.csum_block_size > 0 && alloc_dyn_data ? dirty.dyn_data : NULL); + uint8_t *bmp_ptr = (alloc_dyn_data ? (uint8_t*)dirty.dyn_data + sizeof(int) : (uint8_t*)&dirty.dyn_data); if (!result_version) { @@ -951,8 +950,7 @@ int blockstore_impl_t::read_bitmap(object_id oid, uint64_t target_version, void *result_version = dirty_it->first.version; if (bitmap) { - size_t dyn_size = dsk.dirty_dyn_size(dirty_it->second.offset, dirty_it->second.len); - void *dyn_ptr = (dyn_size > sizeof(void*) + void *dyn_ptr = (alloc_dyn_data ? (uint8_t*)dirty_it->second.dyn_data + sizeof(int) : (uint8_t*)&dirty_it->second.dyn_data); memcpy(bitmap, dyn_ptr, dsk.clean_entry_bitmap_size); } diff --git a/src/blockstore_rollback.cpp b/src/blockstore_rollback.cpp index 243186ad..50b6eb88 100644 --- a/src/blockstore_rollback.cpp +++ b/src/blockstore_rollback.cpp @@ -241,10 +241,7 @@ void blockstore_impl_t::free_dirty_dyn_data(dirty_entry & e) { if (e.dyn_data) { - size_t dyn_size = dsk.dirty_dyn_size(e.offset, e.len); - if (dyn_size > sizeof(void*) && - (!journal.inmemory || e.dyn_data < journal.buffer || - e.dyn_data >= (uint8_t*)journal.buffer + journal.len) && + if (alloc_dyn_data && --*((int*)e.dyn_data) == 0) // refcount { // dyn_data contains the bitmap and checksums diff --git a/src/blockstore_sync.cpp b/src/blockstore_sync.cpp index c8bb5243..4fda01f5 100644 --- a/src/blockstore_sync.cpp +++ b/src/blockstore_sync.cpp @@ -132,7 +132,7 @@ int blockstore_impl_t::continue_sync(blockstore_op_t *op) je->offset = dirty_entry.offset; je->len = dirty_entry.len; je->location = dirty_entry.location; - memcpy((void*)(je+1), (dyn_size > sizeof(void*) + memcpy((void*)(je+1), (alloc_dyn_data ? (uint8_t*)dirty_entry.dyn_data+sizeof(int) : (uint8_t*)&dirty_entry.dyn_data), dyn_size); je->crc32 = je_crc32((journal_entry*)je); journal.crc32_last = je->crc32; diff --git a/src/blockstore_write.cpp b/src/blockstore_write.cpp index 142c0685..16c9ad4d 100644 --- a/src/blockstore_write.cpp +++ b/src/blockstore_write.cpp @@ -14,14 +14,14 @@ bool blockstore_impl_t::enqueue_write(blockstore_op_t *op) op->len = 0; } size_t dyn_size = dsk.dirty_dyn_size(op->offset, op->len); - if (!is_del && dyn_size > sizeof(void*)) + if (!is_del && alloc_dyn_data) { // FIXME: Working with `dyn_data` has to be refactored somehow but I first have to decide how :) // +sizeof(int) = refcount dyn = calloc_or_die(1, dyn_size+sizeof(int)); *((int*)dyn) = 1; } - uint8_t *dyn_ptr = (uint8_t*)(dyn_size > sizeof(void*) ? dyn+sizeof(int) : &dyn); + uint8_t *dyn_ptr = (uint8_t*)(alloc_dyn_data ? dyn+sizeof(int) : &dyn); uint64_t version = 1; if (dirty_db.size() > 0) { @@ -42,7 +42,7 @@ bool blockstore_impl_t::enqueue_write(blockstore_op_t *op) : ((dirty_it->second.state & BS_ST_WORKFLOW_MASK) == BS_ST_WAIT_BIG); if (!is_del && !deleted) { - void *dyn_from = dsk.dirty_dyn_size(dirty_it->second.offset, dirty_it->second.len) > sizeof(void*) + void *dyn_from = alloc_dyn_data ? (uint8_t*)dirty_it->second.dyn_data + sizeof(int) : (uint8_t*)&dirty_it->second.dyn_data; memcpy(dyn_ptr, dyn_from, dsk.clean_entry_bitmap_size); } @@ -120,7 +120,7 @@ bool blockstore_impl_t::enqueue_write(blockstore_op_t *op) printf("Write %lx:%lx v%lu requested, but we already have v%lu\n", op->oid.inode, op->oid.stripe, op->version, version); #endif op->retval = -EEXIST; - if (!is_del && dyn_size > sizeof(void*)) + if (!is_del && alloc_dyn_data) { free(dyn); } @@ -464,7 +464,7 @@ int blockstore_impl_t::dequeue_write(blockstore_op_t *op) je->len = op->len; je->data_offset = journal.next_free; je->crc32_data = dsk.csum_block_size ? 0 : crc32c(0, op->buf, op->len); - memcpy((void*)(je+1), (dyn_size > sizeof(void*) + memcpy((void*)(je+1), (alloc_dyn_data ? (uint8_t*)dirty_it->second.dyn_data+sizeof(int) : (uint8_t*)&dirty_it->second.dyn_data), dyn_size); je->crc32 = je_crc32((journal_entry*)je); journal.crc32_last = je->crc32; @@ -560,7 +560,7 @@ resume_2: je->offset = op->offset; je->len = op->len; je->location = dirty_it->second.location; - memcpy((void*)(je+1), (dyn_size > sizeof(void*) + memcpy((void*)(je+1), (alloc_dyn_data ? (uint8_t*)dirty_it->second.dyn_data+sizeof(int) : (uint8_t*)&dirty_it->second.dyn_data), dyn_size); je->crc32 = je_crc32((journal_entry*)je); journal.crc32_last = je->crc32;