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.

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

Set specific sliced pages for read-only threads in EOS VM OC Tierup #1498

Merged
merged 5 commits into from
Aug 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class eosvmoc_runtime : public eosio::chain::wasm_runtime_interface {

// Defined in eos-vm-oc.cpp. Used for non-main thread in multi-threaded execution
thread_local static std::unique_ptr<eosvmoc::executor> exec_thread_local;
thread_local static eosvmoc::memory mem_thread_local;
thread_local static std::unique_ptr<eosvmoc::memory> mem_thread_local;
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ class memory {
static constexpr uintptr_t first_intrinsic_offset = cb_offset + 8u;
// The maximum amount of data that PIC code can include in the prologue
static constexpr uintptr_t max_prologue_size = mutable_global_size + table_size;
// Number of slices for read-only threads.
// Use a small number to save upfront virtual memory consumption.
// Memory uses beyond this limit will be handled by mprotect.
static constexpr uint32_t sliced_pages_for_ro_thread = 10;

// Changed from -cb_offset == EOS_VM_OC_CONTROL_BLOCK_OFFSET to get around
// of compile warning about comparing integers of different signedness
Expand Down
15 changes: 9 additions & 6 deletions libraries/chain/wasm_interface_collection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,29 @@ namespace eosio::chain {

#ifdef EOSIO_EOS_VM_OC_RUNTIME_ENABLED
struct eosvmoc_tier {
// Called from main thread
eosvmoc_tier(const std::filesystem::path& d, const eosvmoc::config& c, const chainbase::database& db)
: cc(d, c, db) {
// construct exec for the main thread
init_thread_local_data();
// Construct exec and mem for the main thread
exec = std::make_unique<eosvmoc::executor>(cc);
mem = std::make_unique<eosvmoc::memory>(wasm_constraints::maximum_linear_memory/wasm_constraints::wasm_page_size);
}

// Support multi-threaded execution.
// Called from read-only threads
void init_thread_local_data() {
exec = std::make_unique<eosvmoc::executor>(cc);
mem = std::make_unique<eosvmoc::memory>(eosvmoc::memory::sliced_pages_for_ro_thread);
}

eosvmoc::code_cache_async cc;

// Each thread requires its own exec and mem.
thread_local static std::unique_ptr<eosvmoc::executor> exec;
thread_local static eosvmoc::memory mem;
thread_local static std::unique_ptr<eosvmoc::memory> mem;
Copy link
Member

Choose a reason for hiding this comment

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

(obviously not in this PR) but I'd really like for us to noodle on how to move away from these thread_locals, they have dangerous lifetime semantics imo. In this particular example, when eosvmoc_tier is destroyed, cc is destroyed, but the exec created with a reference to that cc lives on. So if executor's dtor (which will run well after eosvmoc_tier's dtor) touches the code_cache_async that was passed to it via reference: bad news! It's a real trap -- passing a reference to some object to the ctor of an object implies that reference will be valid for the lifetime of the object.. but it's not true here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. I will be looking into moving away from those thread_locals.

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 created #1503 to track this. Will work on it soon.

};

thread_local std::unique_ptr<eosvmoc::executor> eosvmoc_tier::exec{};
thread_local eosvmoc::memory eosvmoc_tier::mem{wasm_constraints::maximum_linear_memory / wasm_constraints::wasm_page_size};
thread_local std::unique_ptr<eosvmoc::memory> eosvmoc_tier::mem{};
#endif

wasm_interface_collection::wasm_interface_collection(wasm_interface::vm_type vm, wasm_interface::vm_oc_enable eosvmoc_tierup,
Expand Down Expand Up @@ -71,7 +74,7 @@ void wasm_interface_collection::apply(const digest_type& code_hash, const uint8_
if (cd) {
if (!context.is_applying_block()) // read_only_trx_test.py looks for this log statement
tlog("${a} speculatively executing ${h} with eos vm oc", ("a", context.get_receiver())("h", code_hash));
eosvmoc->exec->execute(*cd, eosvmoc->mem, context);
eosvmoc->exec->execute(*cd, *eosvmoc->mem, context);
return;
}
}
Expand Down
10 changes: 4 additions & 6 deletions libraries/chain/webassembly/runtimes/eos-vm-oc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class eosvmoc_instantiated_module : public wasm_instantiated_module_interface {
if ( is_main_thread() )
_eosvmoc_runtime.exec.execute(*cd, _eosvmoc_runtime.mem, context);
else
_eosvmoc_runtime.exec_thread_local->execute(*cd, _eosvmoc_runtime.mem_thread_local, context);
_eosvmoc_runtime.exec_thread_local->execute(*cd, *_eosvmoc_runtime.mem_thread_local, context);
}

const digest_type _code_hash;
Expand All @@ -57,12 +57,10 @@ std::unique_ptr<wasm_instantiated_module_interface> eosvmoc_runtime::instantiate

void eosvmoc_runtime::init_thread_local_data() {
exec_thread_local = std::make_unique<eosvmoc::executor>(cc);
mem_thread_local = std::make_unique<eosvmoc::memory>(eosvmoc::memory::sliced_pages_for_ro_thread);
}

thread_local std::unique_ptr<eosvmoc::executor> eosvmoc_runtime::exec_thread_local {};
// Set sliced_pages_for_ro_thread to a small number to save upfront virtual memory
// consumption. Usage beyond this limit will be handled by mprotect.
constexpr uint32_t sliced_pages_for_ro_thread = 10;
thread_local eosvmoc::memory eosvmoc_runtime::mem_thread_local{sliced_pages_for_ro_thread};
thread_local std::unique_ptr<eosvmoc::executor> eosvmoc_runtime::exec_thread_local{};
thread_local std::unique_ptr<eosvmoc::memory> eosvmoc_runtime::mem_thread_local{};

}}}}
30 changes: 29 additions & 1 deletion tests/read_only_trx_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import time
import signal
import threading
import os
import platform

from TestHarness import Account, Cluster, ReturnType, TestHelper, Utils, WalletMgr
from TestHarness.TestHelper import AppArgs
Expand Down Expand Up @@ -109,6 +111,9 @@ def startCluster():
specificExtraNodeosArgs[pnodes]+=" --read-only-threads "
specificExtraNodeosArgs[pnodes]+=str(args.read_only_threads)
if args.eos_vm_oc_enable:
if platform.system() != "Linux":
Print("OC not run on Linux. Skip the test")
exit(True) # Do not fail the test
specificExtraNodeosArgs[pnodes]+=" --eos-vm-oc-enable "
specificExtraNodeosArgs[pnodes]+=args.eos_vm_oc_enable
if args.wasm_runtime:
Expand All @@ -132,6 +137,29 @@ def startCluster():
found = producerNode.findInLog(f"executing {eosioCodeHash} with eos vm oc")
assert( found or (noOC and not found) )

if args.eos_vm_oc_enable:
verifyOcVirtualMemory()

def verifyOcVirtualMemory():
try:
with open(f"/proc/{apiNode.pid}/statm") as f:
Copy link
Member

Choose a reason for hiding this comment

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

I realize it doesn't matter now since OC is linux only, but it might be nice to skip this test on non-Linux if that's easy to do.

data = f.read().split()
vmPages = int(data[0])
pageSize = os.sysconf("SC_PAGESIZE")
actualVmSize = vmPages * pageSize

# When OC tierup is enabled, virtual memory used by IC is around
# 529 slices * 8GB (for main thread) + numReadOnlyThreads * 11 slices * 8GB
# This test verifies virtual memory taken by one read-only thread
# is not in the order of 1TB.
otherGB = 1000 # add 1TB for virtual memory used by others
expectedVmSize = ((529 * 8) + (args.read_only_threads * 88) + otherGB) * 1024 * 1024 * 1024
Utils.Print(f"pid: {apiNode.pid}, actualVmSize: {actualVmSize}, expectedVmSize: {expectedVmSize}")
assert(actualVmSize < expectedVmSize)
except FileNotFoundError:
Utils.Print(f"/proc/{apiNode.pid}/statm not found")
assert(False)

def deployTestContracts():
Utils.Print("create test accounts")
testAccount = Account(testAccountName)
Expand Down Expand Up @@ -348,4 +376,4 @@ def runEverythingParallel():
TestHelper.shutdown(cluster, walletMgr, testSuccessful, dumpErrorDetails)

errorCode = 0 if testSuccessful else 1
exit(errorCode)
exit(errorCode)