P2P: Improve performance by not serializing and deserializing blocks from block log

[linh2931]

Currently, when a peer asks for a block, if the block is retrieved from the block log, net plugin

• reads the block form the block log and deserializes it
• then serializes it before sending it to the peer

This can be expensive and add up to considerable CPU, see #611.

Those conversions are not necessary, as the serialization format in block log is the same as on P2P.

This PR adds read_serialized_block_by_num() to block_log and fetch_serialized_block_by_number() to controller, and updates net plugin to use the new interface.

Resolves #612

[ericpassmore]

Note:start
category: Other
component: P2P
summary: Improve P2P performance by maintaining serialized format from block log to peer syncing.
Note:end

[heifner]

Still reviewing, but thought I would get these comments out so you know why PR #803 was created.

[greg7mdp]

minor detail, I think it might be cleaner to declare the return value in the beginning and use it instead of intermadiate variables, as below, but no need to change:

         block_pos_size_t get_block_position_and_size(uint32_t block_num) {
            block_pos_size_t bps { .position = get_block_pos(block_num) };

            if (bps.position == block_log::npos)
               return bps;

            assert(head);
            uint32_t last_block_num = block_header::num_from_id(head->id);
            EOS_ASSERT(block_num <= last_block_num, block_log_exception,
                       "block_num ${bn} should not be greater than last_block_num ${lbn}",
                       ("bn", block_num)("lbn", last_block_num));

            constexpr uint32_t block_pos_size = sizeof(uint64_t); // size of block position field in the block log file

            if (block_num < last_block_num) {
               // current block is not the last block in the log file.
               uint64_t next_block_pos = get_block_pos(block_num + 1);

               EOS_ASSERT(next_block_pos > bps.position + block_pos_size, block_log_exception,
                          "next block position ${np} should be greater than current block position ${p} plus block position field size ${bps}",
                          ("np", next_block_pos)("p", bps.position)("bps", block_pos_size));

               bps.size = next_block_pos - bps.position - block_pos_size;
            } else {
               // current block is the last block in the file.

               block_file.seek_end(0);
               auto file_size = block_file.tellp();
               EOS_ASSERT(file_size > bps.position + block_pos_size, block_log_exception,
                          "block log file size ${fs} should be greater than current block position ${p} plus block position field size ${bps}",
                          ("fs", file_size)("p", bps.position)("bps", block_pos_size));

               bps.size = file_size - bps.position - block_pos_size;
            }

            return bps;
         }

[linh2931]

I think constructing return value right before returning makes it easier to see what to be returned.

[greg7mdp]

Why a std::optional here to mean not found, while we used
return block_pos_size_t {.position = block_log::npos, .size = 0}; in block_log.cpp:626?

[linh2931]

Was trying to follow existing method patterns in the two files: get_block_position in log_catalog.hpp while get_block_pos in block_log.cpp returns block_log::npos.