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

OC tierup compile in order #1342

Merged
merged 5 commits into from
Jun 29, 2023
Merged

OC tierup compile in order #1342

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0997826
GH-1251 Use deque instead of unordered_set to process contracts in or…
heifner Jun 24, 2023
fec0a7a
GH-1251 pass by reference
heifner Jun 26, 2023
57a1d71
GH-1251 Use multiindex, process in order of last used, prioritize eos…
heifner Jun 26, 2023
9cdcef8
GH-1251 Remove MRU, just go with FIFO
heifner Jun 26, 2023
f51599c
GH-1251 Remove chain::config use from eos-vm-oc
heifner Jun 26, 2023
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
8 changes: 4 additions & 4 deletions libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/code_cache.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct config;

class code_cache_base {
public:
code_cache_base(const std::filesystem::path data_dir, const eosvmoc::config& eosvmoc_config, const chainbase::database& db);
code_cache_base(const std::filesystem::path& data_dir, const eosvmoc::config& eosvmoc_config, const chainbase::database& db);
~code_cache_base();

const int& fd() const { return _cache_fd; }
Expand Down Expand Up @@ -80,7 +80,7 @@ class code_cache_base {

//these are really only useful to the async code cache, but keep them here so
//free_code can be shared
std::unordered_set<code_tuple> _queued_compiles;
deque<code_tuple> _queued_compiles;
Copy link
Member

@spoonincode spoonincode Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm worried about the performance ramification of switching this from constant logn time look up to linear time lookup. The lookup occurs every time an action is executed and when someone starts fresh there could be hundreds or thousands of entries that need to be searched through for every action (until compilation settles down).

I think maybe instead we should have a second

std::unordered_set<code_tuple> _high_priority_queued_compiles;

Copy link
Member Author

Choose a reason for hiding this comment

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

One issue I saw was that compiling was according to code_hash sort order instead of order they came in. This preserves the order. If worried about performance, maybe we should use a multindex instead for this which would allow for keeping track of order, last used, prioritize eosio.*, and provide fast lookup.

Copy link
Member

Choose a reason for hiding this comment

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

yeah if you want to maintain compilation order, a multi_index sounds good

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

std::unordered_map<code_tuple, bool> _outstanding_compiles_and_poison;

size_t _free_bytes_eviction_threshold;
Expand All @@ -95,13 +95,13 @@ class code_cache_base {

class code_cache_async : public code_cache_base {
public:
code_cache_async(const std::filesystem::path data_dir, const eosvmoc::config& eosvmoc_config, const chainbase::database& db);
code_cache_async(const std::filesystem::path& data_dir, const eosvmoc::config& eosvmoc_config, const chainbase::database& db);
~code_cache_async();

//If code is in cache: returns pointer & bumps to front of MRU list
//If code is not in cache, and not blacklisted, and not currently compiling: return nullptr and kick off compile
//otherwise: return nullptr
const code_descriptor* const get_descriptor_for_code(const digest_type& code_id, const uint8_t& vm_version, bool is_write_window, get_cd_failure& failure);
const code_descriptor* const get_descriptor_for_code(const account_name& receiver, const digest_type& code_id, const uint8_t& vm_version, bool is_write_window, get_cd_failure& failure);

private:
std::thread _monitor_reply_thread;
Expand Down
2 changes: 1 addition & 1 deletion libraries/chain/wasm_interface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ namespace eosio { namespace chain {
const chain::eosvmoc::code_descriptor* cd = nullptr;
chain::eosvmoc::code_cache_base::get_cd_failure failure = chain::eosvmoc::code_cache_base::get_cd_failure::temporary;
try {
cd = my->eosvmoc->cc.get_descriptor_for_code(code_hash, vm_version, context.control.is_write_window(), failure);
cd = my->eosvmoc->cc.get_descriptor_for_code(context.get_receiver(), code_hash, vm_version, context.control.is_write_window(), failure);
}
catch(...) {
//swallow errors here, if EOS VM OC has gone in to the weeds we shouldn't bail: continue to try and run baseline
Expand Down
18 changes: 12 additions & 6 deletions libraries/chain/webassembly/runtimes/eos-vm-oc/code_cache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <eosio/chain/webassembly/eos-vm-oc/intrinsic.hpp>
#include <eosio/chain/webassembly/eos-vm-oc/compile_monitor.hpp>
#include <eosio/chain/exceptions.hpp>
#include <eosio/chain/config.hpp>

#include <unistd.h>
#include <sys/syscall.h>
Expand Down Expand Up @@ -38,7 +39,7 @@ static constexpr size_t descriptor_ptr_from_file_start = header_offset + offseto

static_assert(sizeof(code_cache_header) <= header_size, "code_cache_header too big");

code_cache_async::code_cache_async(const std::filesystem::path data_dir, const eosvmoc::config& eosvmoc_config, const chainbase::database& db) :
code_cache_async::code_cache_async(const std::filesystem::path& data_dir, const eosvmoc::config& eosvmoc_config, const chainbase::database& db) :
code_cache_base(data_dir, eosvmoc_config, db),
_result_queue(eosvmoc_config.threads * 2),
_threads(eosvmoc_config.threads)
Expand Down Expand Up @@ -106,7 +107,7 @@ std::tuple<size_t, size_t> code_cache_async::consume_compile_thread_queue() {
}


const code_descriptor* const code_cache_async::get_descriptor_for_code(const digest_type& code_id, const uint8_t& vm_version, bool is_write_window, get_cd_failure& failure) {
const code_descriptor* const code_cache_async::get_descriptor_for_code(const account_name& receiver, const digest_type& code_id, const uint8_t& vm_version, bool is_write_window, get_cd_failure& failure) {
Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine to leave as is, but for increased abstraction maybe this just takes bool high_priority instead, and the receiver.prefix() == chain::config::system_account_name check is then left outside of any eos-vm-oc code

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 like that. Didn't feel right including chain::config here.

//if there are any outstanding compiles, process the result queue now
//When app is in write window, all tasks are running sequentially and read-only threads
//are not running. Safe to update cache entries.
Expand Down Expand Up @@ -156,13 +157,16 @@ const code_descriptor* const code_cache_async::get_descriptor_for_code(const dig
it->second = false;
return nullptr;
}
if(_queued_compiles.find(ct) != _queued_compiles.end()) {
if(std::find(_queued_compiles.cbegin(), _queued_compiles.cend(), ct) != _queued_compiles.end()) {
failure = get_cd_failure::temporary; // Compile might not be done yet
return nullptr;
}

if(_outstanding_compiles_and_poison.size() >= _threads) {
_queued_compiles.emplace(ct);
if (receiver.prefix() == chain::config::system_account_name)
_queued_compiles.push_front(ct);
else
_queued_compiles.push_back(ct);
failure = get_cd_failure::temporary; // Compile might not be done yet
return nullptr;
}
Expand Down Expand Up @@ -221,7 +225,7 @@ const code_descriptor* const code_cache_sync::get_descriptor_for_code_sync(const
return &*_cache_index.push_front(std::move(std::get<code_descriptor>(result.result))).first;
}

code_cache_base::code_cache_base(const std::filesystem::path data_dir, const eosvmoc::config& eosvmoc_config, const chainbase::database& db) :
code_cache_base::code_cache_base(const std::filesystem::path& data_dir, const eosvmoc::config& eosvmoc_config, const chainbase::database& db) :
_db(db),
_cache_file_path(data_dir/"code_cache.bin")
{
Expand Down Expand Up @@ -383,7 +387,9 @@ void code_cache_base::free_code(const digest_type& code_id, const uint8_t& vm_ve
}

//if it's in the queued list, erase it
_queued_compiles.erase({code_id, vm_version});
auto i = std::find(_queued_compiles.cbegin(), _queued_compiles.cend(), code_tuple{code_id, vm_version});
if (i != _queued_compiles.cend())
_queued_compiles.erase(i);

//however, if it's currently being compiled there is no way to cancel the compile,
//so instead set a poison boolean that indicates not to insert the code in to the cache
Expand Down