StackSpace: use std::aligned_alloc() #907
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #907 +/- ##
=======================================
Coverage 98.26% 98.26%
=======================================
Files 131 131
Lines 15709 15715 +6
=======================================
+ Hits 15437 15443 +6
Misses 272 272
Flags with carried forward coverage won't be shown. Click here to find out more.
|
chfast
force-pushed
the
stack_space_aligned_alloc
branch
from
fc9b5c1
to
a8b779c
Compare
lib/evmone/execution_state.hpp
Outdated
|
|
||
| /// The storage allocated for maximum possible number of items. | ||
| /// Items are aligned to 256 bits for better packing in cache lines. | ||
| std::unique_ptr<uint256, Deleter> m_stack_space{allocate()}; |
There was a problem hiding this comment.
I'd put the allocate() call in a constructor.
| /// Returns the pointer to the "bottom", i.e. below the stack space. | ||
| [[nodiscard, clang::no_sanitize("bounds")]] uint256* bottom() noexcept | ||
| { | ||
| return m_stack_space - 1; | ||
| return m_stack_space.get() - 1; |
There was a problem hiding this comment.
Why does it need to go outside the memory bounds?
There was a problem hiding this comment.
It is the convention we use currently: we keep the pointer to the top element on the stack. Therefore its starting position (stack empty) is one element below the allocated memory.
Use `std::aligned_alloc()` (and `_aligned_malloc()` on MSVC) to allocate the memory for EVM stack. This removes unnecessary initialization of the memory to zero (closes #403). The memory alignment is set to 32 bytes (as previously) but allows transitioning to "over-aligned" allocation for additional optimization (see #781).
chfast
force-pushed
the
stack_space_aligned_alloc
branch
from
a8b779c
to
6779073
Compare
| public: | ||
| /// The maximum number of EVM stack items. | ||
| static constexpr auto limit = 1024; | ||
|
|
||
| StackSpace() noexcept : m_stack_space{allocate()} {} |
There was a problem hiding this comment.
I wonder if
std::unique_ptr<uint256[]>(new (std::align_val_t(sizeof(uint256)) uint256[limit]);achieves the same.
There was a problem hiding this comment.
It looks like, however this specifies the array element alignment, so I wonder what happens if you specify 64 for uint256. I plan to align the allocation to 2048*32.
Use
std::aligned_alloc()(and_aligned_malloc()on MSVC) to allocate the memory for EVM stack.This removes unnecessary initialization of the memory to zero (closes #403).
The memory alignment is set to 32 bytes (as previously) but allows transitioning to "over-aligned" allocation for additional optimization
(see #781).