Skip to content
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: support IC low Wasm memory hook #4849

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

luc-blaeser
Copy link
Contributor

@luc-blaeser luc-blaeser commented Jan 15, 2025

Supporting Low Memory Hook in Motoko

The IC allows to implement a low memory hook, which is a warning trigger when main memory is becoming scarce.

For this purpose, a Motoko actor or actor class instance can implement the system function lowmemory(). This system function is scheduled when canister's free main memory space has fallen below the defined threshold wasm_memory_threshold, that is is part of the canister settings. In Motoko, lowmemory() implements the canister_on_low_wasm_memory hook defined in the IC specification.

Reference: dfinity/portal#3761

Example of implementing the low memory hook:

actor {
    system func lowmemory() : async* () {
        Debug.print("Low memory!");
    }
}

The following properties should be considered when using the low memory hook:

  • The execution of lowmemory happens with a certain delay, as it is scheduled as a separate asynchronous message that runs after the message in which the threshold was crossed.
  • Once executed, lowmemory will only be triggered again when the main memory free space grows above the threshold (e.g. by lowering the threshold or shrinking the main memory through canister reinstallation) and then again falls below the threshold.
  • Traps or unhandled errors in lowmemory are ignored. They only revert the changes done in lowmemory.
  • Due to its async* return type, the lowmemory function may send further messages and await results.

@luc-blaeser luc-blaeser requested a review from a team as a code owner January 15, 2025 08:57
@luc-blaeser luc-blaeser self-assigned this Jan 15, 2025
Copy link

github-actions bot commented Jan 15, 2025

Comparing from 607ecfd to e349c26:
In terms of gas, 5 tests regressed and the mean change is +0.6%.
In terms of size, no changes are observed in 5 tests.

test/fail/ok/M0129.tc.ok Outdated Show resolved Hide resolved
@luc-blaeser luc-blaeser requested a review from crusso January 15, 2025 17:45
@ggreif ggreif changed the title Support IC Low Wasm Memory Hook feat: support IC Low Wasm Memory Hook Jan 15, 2025
@ggreif ggreif changed the title feat: support IC Low Wasm Memory Hook feat: support IC low Wasm memory hook Jan 15, 2025
test/run-drun/low-memory.mo Outdated Show resolved Hide resolved
crusso
crusso previously approved these changes Jan 16, 2025
Copy link
Contributor

@crusso crusso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could consider allowing the return type to be async* but the method would need to force the async* somehow. That would avoid another scheduling hop.

Do we want merge this now or wait for the IC implementation to be fixed?

@crusso crusso self-requested a review January 16, 2025 11:09
Co-authored-by: Claudio Russo <claudio@dfinity.org>
@luc-blaeser
Copy link
Contributor Author

I guess we could consider allowing the return type to be async* but the method would need to force the async* somehow. That would avoid another scheduling hop.

Do we want merge this now or wait for the IC implementation to be fixed?

I changed it to async*. Thanks for the suggestion.

I think we can merge it as soon as it is ready from our side. The IC fix can come later and would be automatically become effective.

ggreif
ggreif previously approved these changes Jan 17, 2025
Copy link
Contributor

@ggreif ggreif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added one suggestion, otherwise LGTM!

Co-authored-by: Gabor Greif <gabor@dfinity.org>
@luc-blaeser
Copy link
Contributor Author

Thanks for reviewing!

@luc-blaeser luc-blaeser added automerge-squash When ready, merge (using squash) and removed automerge-squash When ready, merge (using squash) labels Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants