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

fix: do not call done() in stable_restore() #216

Merged
merged 1 commit into from
Feb 24, 2022
Merged

fix: do not call done() in stable_restore() #216

merged 1 commit into from
Feb 24, 2022

Conversation

roman-kashitsyn
Copy link
Contributor

This change removes the call to decoder.done() in stable_restore().

Rationale:

  • done() almost always returns an error because the input is likely
    to contain trailing zero bytes.

  • Calling done() has no effect on the return value.

  • The error that done() constructs contains the dump of the whole state,
    which causes the decoder to allocate huge amounts of memory.
    All of these allocations are unnecessary because stable_restore()
    throws away the error right away.

Fixes #212.

This change removes the call to decoder.done() in stable_restore().

Rationale:

 * done() almost always returns an error because the input is likely
   to contain trailing zero bytes.

 * Calling done() has no effect on the return value.

 * The error that done() constructs contains the dump of the whole state,
   which causes the decoder to allocate huge amounts of memory.
   All of these allocations are unnecessary because stable_restore()
   throws away the error right away.

Fixes #212.
@lwshang lwshang merged commit 5a0206b into main Feb 24, 2022
@lwshang lwshang deleted the roman-fix-212 branch February 24, 2022 16:11
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.

Huge heap memory increase (10x) when calling ic_cdk::storage::stable_restore()
3 participants