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.
| [[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 
| /// 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).