Execution context stack space #777
Conversation
Codecov Report
@@ Coverage Diff @@
## master #777 +/- ##
==========================================
- Coverage 99.24% 99.18% -0.07%
==========================================
Files 79 77 -2
Lines 11962 10948 -1014
==========================================
- Hits 11872 10859 -1013
+ Misses 90 89 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
chfast
force-pushed
the
execution_context_stack_space
branch
from
fef2074 to
47111ae
Compare
chfast
force-pushed
the
execution_context_stack_space
branch
2 times, most recently
from
0e5287c to
85c5b53
Compare
|
Unit tests will be updated later. |
|
|
||
| OperandStack stack(args, func_type.inputs.size(), code.local_count, | ||
| static_cast<size_t>(code.max_stack_height)); | ||
| const auto local_ctx = ctx.create_local_context(required_stack_space); |
There was a problem hiding this comment.
Should handle somehow that this can throw now.
There was a problem hiding this comment.
It throws before too.
There was a problem hiding this comment.
To be precise: now create_local_context() throws, previously OperandStack was throwing (now noexcept).
Someone should handle this separately. But there is also easy solution: have fixed shared stack space. In case of out-of-space we can trap the execution.
There was a problem hiding this comment.
Catching bad_alloc and trapping would also be easy
There was a problem hiding this comment.
But this is non-deterministic execution.
| prev_stack_space_segment = m_shared_ctx.stack_space_segment; | ||
| const auto new_segment_size = | ||
| std::max(DefaultStackSpaceSegmentSize, required_stack_space); | ||
| m_shared_ctx.stack_space_segment = new Value[new_segment_size]; |
There was a problem hiding this comment.
It would allow a performance degrading behaviour like growing stack to almost DefaultStackSpaceSegmentSize, then calling a function in a loop that goes over the limit, allocates new segment and immediately deallocates after return, and that's repeated in a loop.
Should this be a concern? I can imagine mitigation with increasing allocation size each time, and keeping the last allocation size in m_shared_ctx.
There was a problem hiding this comment.
It is a concern, but the same as before (where stack space was dynamically allocated in some cases).
I don't think the increasing allocation size is the solution - it will still continue to deallocating new segment, now bigger each time.
The solution I considered is never deallocate segments. They work as linked list of buffers. However, such solution might require more housekeeping.
chfast
force-pushed
the
execution_context_stack_space
branch
from
85c5b53 to
31f44e0
Compare
chfast
force-pushed
the
execution_context_stack_space
branch
from
31f44e0 to
2d29813
Compare
This implements segmented stack space inside ExecutionContext. I will add more documentation how it works later.
Replaces #529 and #572.
Generally, having unlimited stack space is PITA, but doable.
Before this lands I propose to generalize
ExecutionContext:execution_context.hpp,Guardto something likeLocalContext,increment_call_depth()to something likecreate_local_context().