Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Adjustable stack size for EVM #2483

Merged
merged 5 commits into from
Oct 6, 2016
Merged

Adjustable stack size for EVM #2483

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
33c8ac4
stack size for io workers & evm threshold
NikVolf Oct 5, 2016
f2186f5
rust way to remember stack size
NikVolf Oct 5, 2016
be2307f
right value
NikVolf Oct 6, 2016
841401e
24kb size
NikVolf Oct 6, 2016
7b956a8
some stack reduction
NikVolf Oct 6, 2016
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
9 changes: 6 additions & 3 deletions ethcore/src/executive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ use trace::{FlatTrace, Tracer, NoopTracer, ExecutiveTracer, VMTrace, VMTracer, E
use crossbeam;
pub use types::executed::{Executed, ExecutionResult};

/// Max depth to avoid stack overflow (when it's reached we start a new thread with VM)
/// Roughly estimate what stack size each level of evm depth will use
/// TODO [todr] We probably need some more sophisticated calculations here (limit on my machine 132)
/// Maybe something like here: `https://github.com/ethereum/libethereum/blob/4db169b8504f2b87f7d5a481819cfb959fc65f6c/libethereum/ExtVM.cpp`
const MAX_VM_DEPTH_FOR_THREAD: usize = 64;
const STACK_SIZE_PER_DEPTH: usize = 256*1024;
Copy link
Collaborator

Choose a reason for hiding this comment

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

256kb per recursion frame? That's too much. Before the change it was 512k / 64 = 8kb
512k is the minimum stack size that we supported

Copy link
Contributor Author

@NikVolf NikVolf Oct 6, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

before change it was 2MB/64 = 32kb
i just used 16MB/64 instead, my bad


/// Returns new address created from address and given nonce.
pub fn contract_address(address: &Address, nonce: &U256) -> Address {
Expand Down Expand Up @@ -212,8 +212,11 @@ impl<'a> Executive<'a> {
tracer: &mut T,
vm_tracer: &mut V
) -> evm::Result<U256> where T: Tracer, V: VMTracer {

let depth_threshold = ::io::LOCAL_STACK_SIZE.with(|sz| sz.get() / STACK_SIZE_PER_DEPTH);

// Ordinary execution - keep VM in same thread
if (self.depth + 1) % MAX_VM_DEPTH_FOR_THREAD != 0 {
if (self.depth + 1) % depth_threshold != 0 {
let vm_factory = self.vm_factory;
let mut ext = self.as_externalities(OriginInfo::from(&params), unconfirmed_substate, output_policy, tracer, vm_tracer);
trace!(target: "executive", "ext.schedule.have_delegate_call: {}", ext.schedule().have_delegate_call);
Expand Down
2 changes: 2 additions & 0 deletions util/io/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ mod panics;
use mio::{EventLoop, Token};
use std::fmt;

pub use worker::LOCAL_STACK_SIZE;

#[derive(Debug)]
/// IO Error
pub enum IoError {
Expand Down
13 changes: 12 additions & 1 deletion util/io/src/worker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,19 @@ use crossbeam::sync::chase_lev;
use service::{HandlerId, IoChannel, IoContext};
use IoHandler;
use panics::*;
use std::cell::Cell;

use std::sync::{Condvar as SCondvar, Mutex as SMutex};

const STACK_SIZE: usize = 16*1024*1024;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like with 8kb frame the stack of 8mb should be enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this might be so, but previously we used 32kb
see above


thread_local! {
/// Stack size
/// Should be modified if it is changed in Rust since it is no way
/// to know or get it
pub static LOCAL_STACK_SIZE: Cell<usize> = Cell::new(::std::env::var("RUST_MIN_STACK").ok().and_then(|s| s.parse().ok()).unwrap_or(2 * 1024 * 1024));
Copy link
Collaborator

Choose a reason for hiding this comment

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

minimum should be 512k, that's the default on OS X

Copy link
Contributor Author

Choose a reason for hiding this comment

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

512k must be default for the main thread only

this expression exactly the same value which rust uses for spawned threads:
https://github.com/rust-lang/rust/blob/master/src/libstd/sys/common/util.rs#L18

}

pub enum WorkType<Message> {
Readable,
Writable,
Expand Down Expand Up @@ -66,8 +76,9 @@ impl Worker {
deleting: deleting.clone(),
wait_mutex: wait_mutex.clone(),
};
worker.thread = Some(thread::Builder::new().name(format!("IO Worker #{}", index)).spawn(
worker.thread = Some(thread::Builder::new().stack_size(STACK_SIZE).name(format!("IO Worker #{}", index)).spawn(
move || {
LOCAL_STACK_SIZE.with(|val| val.set(STACK_SIZE));
panic_handler.catch_panic(move || {
Worker::work_loop(stealer, channel.clone(), wait, wait_mutex.clone(), deleting)
}).unwrap()
Expand Down