feat: [EXC-1669] Propagate hook execution status to SystemState #667
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dragoljub-duric
commented
Aug 8, 2024
github-actions
bot
added
@execution
@ic-interface-owners
@ic-message-routing-owners
labels
maksymar
approved these changes
Sep 24, 2024
alin-at-dfinity
approved these changes
Sep 24, 2024
dragoljub-duric
deleted the
EXC-1669-implement-checking-for-hook-condition-and-execute-hook
branch
frankdavid
pushed a commit
that referenced
this pull request
Sep 25, 2024
This PR is part of the OnLowWasmMemoryHook effort. Here we implement checking for hook condition and propagating hook status to SystemState. Condition for executing hook, as agreed in interface spec [discussion](dfinity/interface-spec#286 (comment)) is: 1. In the case with memory_allocation `min(memory_allocation - used_stable_memory, wasm_memory_limit) - used_wasm_memory` 2. Without memory allocation `wasm_memory_limit - used_wasm_memory` Hook status can be one of: 1. Condition is not satisfied 3. Ready for execution 4. Executed The hook condition is checked whenever additional execution (stable or Wasm) memory allocation is requested. After this change, we will have all the required information of SystemState and it will be necessary only to schedule hook execution in the following round. This PR will be followed with: 1. [EXC-1724](https://dfinity.atlassian.net/browse/EXC-1724) In which we will refactor `SystemState::task_queue` to be new struct with additional field `on_low_wasm_memory_hook_status`, to ensure that whenever available first task that will be poped form task queue is `OnLowWasmMemoryHook` (excluding paused executions). For that reason `SystemState::on_low_wasm_memory_hook status` is private, and we use `set` and `get`, so it can be easier encapsulated in the future `TaskQueue` struct. 2. [EXC-1725](https://dfinity.atlassian.net/browse/EXC-1725) Schedule and execute `OnLowWasmMemoryHook` [EXC-1724]: https://dfinity.atlassian.net/browse/EXC-1724?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [EXC-1725]: https://dfinity.atlassian.net/browse/EXC-1725?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Alin Sinpalean <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is part of the OnLowWasmMemoryHook effort. Here we implement checking for hook condition and propagating hook status to SystemState.
Condition for executing hook, as agreed in interface spec discussion is:
min(memory_allocation - used_stable_memory, wasm_memory_limit) - used_wasm_memorywasm_memory_limit - used_wasm_memoryHook status can be one of:
The hook condition is checked whenever additional execution (stable or Wasm) memory allocation is requested.
After this change, we will have all the required information of SystemState and it will be necessary only to schedule hook execution in the following round.
This PR will be followed with:
SystemState::task_queueto be new struct with additional fieldon_low_wasm_memory_hook_status, to ensure that whenever available first task that will be poped form task queue isOnLowWasmMemoryHook(excluding paused executions). For that reasonSystemState::on_low_wasm_memory_hook statusis private, and we usesetandget, so it can be easier encapsulated in the futureTaskQueuestruct.OnLowWasmMemoryHook