-
Notifications
You must be signed in to change notification settings - Fork 317
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: [EXC-1669] Propagate hook execution status to SystemState #667
Merged
dragoljub-duric
merged 58 commits into
master
from
EXC-1669-implement-checking-for-hook-condition-and-execute-hook
Sep 24, 2024
Merged
feat: [EXC-1669] Propagate hook execution status to SystemState #667
dragoljub-duric
merged 58 commits into
master
from
EXC-1669-implement-checking-for-hook-condition-and-execute-hook
Sep 24, 2024
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
Sep 20, 2024
maksymar
approved these changes
Sep 24, 2024
Co-authored-by: Alin Sinpalean <58422065+alin-at-dfinity@users.noreply.github.com>
alin-at-dfinity
approved these changes
Sep 24, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you.
dragoljub-duric
deleted the
EXC-1669-implement-checking-for-hook-condition-and-execute-hook
branch
September 24, 2024 20:50
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 <58422065+alin-at-dfinity@users.noreply.github.com>
levifeldman
pushed a commit
to levifeldman/ic
that referenced
this pull request
Oct 1, 2024
…ity#667) 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 <58422065+alin-at-dfinity@users.noreply.github.com>
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_memory
wasm_memory_limit - used_wasm_memory
Hook 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_queue
to 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 status
is private, and we useset
andget
, so it can be easier encapsulated in the futureTaskQueue
struct.OnLowWasmMemoryHook