Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 30 commits into from
Sep 23, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ca100fb
Add read_serialized_block_by_num() and read_serialized_block_by_id()
linh2931 Sep 15, 2024
d1ef5cf
Add fetch_serialized_block_by_number() and fetch_serialized_block_by_…
linh2931 Sep 15, 2024
e65a9b6
Update net_plugin for changes required to handle serialized blocks
linh2931 Sep 15, 2024
81ef718
Verify block number or block ID before returning serialized signed block
linh2931 Sep 16, 2024
907dcdc
Do not hardcode size of block position field in block log file
linh2931 Sep 16, 2024
5211dea
Use the return of retry_read_block_by_num only if it is not nullptr
linh2931 Sep 16, 2024
5ce9f87
Merge branch 'main' into get_serialized_block
linh2931 Sep 16, 2024
b997d30
Make EOS_ASSERT in get_block_num_at stricter
linh2931 Sep 18, 2024
0d0e55b
Improve comments and rename a variable
linh2931 Sep 18, 2024
c39f4f1
Remove the code to validate block number and block ID in a serialized…
linh2931 Sep 18, 2024
12c63d8
Revert the change of moving block_num_at out of block_log_data class
linh2931 Sep 18, 2024
543a1a5
Streamline error handling
linh2931 Sep 18, 2024
cb0112f
More EOS_ASSERTs
linh2931 Sep 19, 2024
abac4c3
Merge branch 'main' into get_serialized_block
linh2931 Sep 19, 2024
55994b0
GH-612 Remove dead code
heifner Sep 19, 2024
246ccfb
GH-612 Missed the no break.
heifner Sep 19, 2024
eaa6161
Refactor to handle split block log files
linh2931 Sep 19, 2024
d15b2d8
Merge pull request #803 from AntelopeIO/p2p-rm-dead-code
heifner Sep 19, 2024
d25f119
Remove read_serialized_block_by_id() which is not longer needed
linh2931 Sep 19, 2024
0a93311
Append serialized block to a passed in buffer to save copying
linh2931 Sep 20, 2024
0f0b1f6
Revert size() change in block_num_at
linh2931 Sep 20, 2024
53fe9a1
remove an unnecessary EOS_ASSERT
linh2931 Sep 20, 2024
9c1c6b8
Merge branch 'main' into get_serialized_block
linh2931 Sep 20, 2024
1340e3b
Revert back to return std::vector<char>
linh2931 Sep 20, 2024
907d9b0
Use EOS_ASSERT for
linh2931 Sep 20, 2024
1888501
use seek_end(0) and tellp to find file size for the already openned b…
linh2931 Sep 20, 2024
a9ae2d8
Use structure binding
linh2931 Sep 21, 2024
d6ef773
Merge branch 'main' into get_serialized_block
linh2931 Sep 21, 2024
b40b50d
Simplify get_block_position_and_size
linh2931 Sep 22, 2024
1075d83
add back peer_dlog in enqueue_block
linh2931 Sep 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 86 additions & 3 deletions libraries/chain/block_log.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,16 @@ namespace eosio { namespace chain {
return block;
}

template <typename Stream>
std::vector<char> read_serialized_block(Stream&& ds, uint64_t block_size) {
std::vector<char> buff;
buff.resize(block_size);

ds.read(buff.data(), block_size);

return buff;
}

template <typename Stream>
signed_block_header read_block_header(Stream&& ds, uint32_t expect_block_num) {
signed_block_header bh;
Expand All @@ -197,6 +207,7 @@ namespace eosio { namespace chain {
return bh;
}


/// Provide the read only view of the blocks.log file
class block_log_data : public chain::log_data_base<block_log_data> {
block_log_preamble preamble;
Expand Down Expand Up @@ -245,10 +256,12 @@ namespace eosio { namespace chain {
// block_id_type previous; //bytes 14:45, low 4 bytes is big endian block number
// of previous block

EOS_ASSERT(position <= size(), block_log_exception, "Invalid block position ${position}",
("position", position));
int blknum_offset = 14;

EOS_ASSERT(position + blknum_offset + sizeof(uint32_t) <= size(), block_log_exception,
"Read outside of file: position ${position}, blknum_offset ${o}, file size ${s}",
("position", position)("o", blknum_offset)("s", size()));

int blknum_offset = 14;
uint32_t prev_block_num = read_data_at<uint32_t>(file, position + blknum_offset);
return fc::endian_reverse_u32(prev_block_num) + 1;
}
Expand Down Expand Up @@ -485,6 +498,7 @@ namespace eosio { namespace chain {
virtual void flush() = 0;

virtual signed_block_ptr read_block_by_num(uint32_t block_num) = 0;
virtual std::vector<char> read_serialized_block_by_num(uint32_t block_num) = 0;
virtual std::optional<signed_block_header> read_block_header_by_num(uint32_t block_num) = 0;

virtual uint32_t version() const = 0;
Expand Down Expand Up @@ -518,6 +532,7 @@ namespace eosio { namespace chain {
void flush() final {}

signed_block_ptr read_block_by_num(uint32_t block_num) final { return {}; };
std::vector<char> read_serialized_block_by_num(uint32_t block_num) final { return {}; };
std::optional<signed_block_header> read_block_header_by_num(uint32_t block_num) final { return {}; };

uint32_t version() const final { return 0; }
Expand Down Expand Up @@ -562,6 +577,7 @@ namespace eosio { namespace chain {
virtual uint32_t working_block_file_first_block_num() { return preamble.first_block_num; }
virtual void post_append(uint64_t pos) {}
virtual signed_block_ptr retry_read_block_by_num(uint32_t block_num) { return {}; }
virtual std::vector<char> retry_read_serialized_block_by_num(uint32_t block_num) { return {}; }
virtual std::optional<signed_block_header> retry_read_block_header_by_num(uint32_t block_num) { return {}; }

void append(const signed_block_ptr& b, const block_id_type& id,
Expand Down Expand Up @@ -603,6 +619,46 @@ namespace eosio { namespace chain {
return pos;
}

block_pos_size_t get_block_position_and_size(uint32_t block_num) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
         }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

uint64_t pos = get_block_pos(block_num);

if (pos == block_log::npos) {
return block_pos_size_t {.position = block_log::npos, .size = 0};
}

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));

uint64_t block_size = 0;
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 > pos + 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", pos)("bps", block_pos_size));

block_size = next_block_pos - pos - 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 > pos + 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", pos)("bps", block_pos_size));

block_size = file_size - pos - block_pos_size;
}

return block_pos_size_t {.position = pos, .size = block_size};
}

signed_block_ptr read_block_by_num(uint32_t block_num) final {
try {
uint64_t pos = get_block_pos(block_num);
Expand All @@ -615,6 +671,18 @@ namespace eosio { namespace chain {
FC_LOG_AND_RETHROW()
}

std::vector<char> read_serialized_block_by_num(uint32_t block_num) final {
try {
auto [ position, size ] = get_block_position_and_size(block_num);
if (position != block_log::npos) {
block_file.seek(position);
return read_serialized_block(block_file, size);
}
return retry_read_serialized_block_by_num(block_num);
}
FC_LOG_AND_RETHROW()
}

std::optional<signed_block_header> read_block_header_by_num(uint32_t block_num) final {
try {
uint64_t pos = get_block_pos(block_num);
Expand Down Expand Up @@ -1032,6 +1100,16 @@ namespace eosio { namespace chain {
return {};
}

std::vector<char> retry_read_serialized_block_by_num(uint32_t block_num) final {
uint64_t block_size = 0;

auto ds = catalog.ro_stream_and_size_for_block(block_num, block_size);
if (ds) {
return read_serialized_block(*ds, block_size);
}
return {};
}

std::optional<signed_block_header> retry_read_block_header_by_num(uint32_t block_num) final {
auto ds = catalog.ro_stream_for_block(block_num);
if (ds)
Expand Down Expand Up @@ -1230,6 +1308,11 @@ namespace eosio { namespace chain {
return my->read_block_by_num(block_num);
}

std::vector<char> block_log::read_serialized_block_by_num(uint32_t block_num) const {
std::lock_guard g(my->mtx);
return my->read_serialized_block_by_num(block_num);
}

std::optional<signed_block_header> block_log::read_block_header_by_num(uint32_t block_num) const {
std::lock_guard g(my->mtx);
return my->read_block_header_by_num(block_num);
Expand Down
8 changes: 8 additions & 0 deletions libraries/chain/controller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5543,6 +5543,14 @@ signed_block_ptr controller::fetch_block_by_number( uint32_t block_num )const {
return my->blog.read_block_by_num(block_num);
} FC_CAPTURE_AND_RETHROW( (block_num) ) }

std::vector<char> controller::fetch_serialized_block_by_number( uint32_t block_num)const { try {
if (signed_block_ptr b = my->fork_db_fetch_block_on_best_branch_by_num(block_num)) {
return fc::raw::pack(*b);
}

return my->blog.read_serialized_block_by_num(block_num);
} FC_CAPTURE_AND_RETHROW( (block_num) ) }

std::optional<signed_block_header> controller::fetch_block_header_by_number( uint32_t block_num )const { try {
auto b = my->fork_db_fetch_block_on_best_branch_by_num(block_num);
if (b)
Expand Down
3 changes: 2 additions & 1 deletion libraries/chain/include/eosio/chain/block_log.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ namespace eosio { namespace chain {
void reset( const genesis_state& gs, const signed_block_ptr& genesis_block );
void reset( const chain_id_type& chain_id, uint32_t first_block_num );

signed_block_ptr read_block_by_num(uint32_t block_num)const;
signed_block_ptr read_block_by_num(uint32_t block_num)const;
std::vector<char> read_serialized_block_by_num(uint32_t block_num)const;
std::optional<signed_block_header> read_block_header_by_num(uint32_t block_num)const;
std::optional<block_id_type> read_block_id_by_num(uint32_t block_num)const;

Expand Down
2 changes: 2 additions & 0 deletions libraries/chain/include/eosio/chain/controller.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,8 @@ namespace eosio::chain {
signed_block_ptr fetch_block_by_number( uint32_t block_num )const;
// thread-safe
signed_block_ptr fetch_block_by_id( const block_id_type& id )const;
// thread-safe, retrieves serialized signed block
std::vector<char> fetch_serialized_block_by_number( uint32_t block_num)const;
// thread-safe
bool block_exists(const block_id_type& id) const;
bool validated_block_exists(const block_id_type& id) const;
Expand Down
34 changes: 34 additions & 0 deletions libraries/chain/include/eosio/chain/log_catalog.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ struct null_verifier {
void verify(const LogData&, const std::filesystem::path&) {}
};

struct block_pos_size_t {
uint64_t position = 0; // start position of the block in block log
uint64_t size = 0; // size of the block
};

template <typename LogData, typename LogIndex, typename LogVerifier = null_verifier>
struct log_catalog {
using block_num_t = uint32_t;
Expand Down Expand Up @@ -159,6 +164,26 @@ struct log_catalog {
}
}

std::optional<block_pos_size_t> get_block_position_and_size(uint32_t block_num) {
std::optional<uint64_t> pos = get_block_position(block_num);

if (!pos) {
return {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

constexpr uint32_t block_pos_size = sizeof(uint64_t);
uint64_t block_size = 0;
assert(block_num <= log_data.last_block_num());
if (block_num < log_data.last_block_num()) {
uint64_t next_block_pos = log_index.nth_block_position(block_num + 1 - log_data.first_block_num());
block_size = next_block_pos - *pos - block_pos_size;
} else {
block_size = log_data.size() - *pos - block_pos_size;
}

return block_pos_size_t { .position = *pos, .size = block_size };
}

fc::datastream<fc::cfile>* ro_stream_for_block(uint32_t block_num) {
auto pos = get_block_position(block_num);
if (pos) {
Expand All @@ -176,6 +201,15 @@ struct log_catalog {
return {};
}

fc::datastream<fc::cfile>* ro_stream_and_size_for_block(uint32_t block_num, uint64_t& block_size) {
auto pos_size = get_block_position_and_size(block_num);
if (pos_size) {
block_size = pos_size->size;
return &log_data.ro_stream_at(pos_size->position);
}
return nullptr;
}

std::optional<block_id_type> id_for_block(uint32_t block_num) {
auto pos = get_block_position(block_num);
if (pos) {
Expand Down
Loading