From d67b80bd1c4a46d87caf0d1fb3aa7f4dbc3fe2a9 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 24 Nov 2023 14:00:39 +0100 Subject: [PATCH 1/9] Remove the concept of "heap pages" from the client --- text/0051-remove-heap-pages.md | 88 ++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 text/0051-remove-heap-pages.md diff --git a/text/0051-remove-heap-pages.md b/text/0051-remove-heap-pages.md new file mode 100644 index 000000000..db33a7789 --- /dev/null +++ b/text/0051-remove-heap-pages.md @@ -0,0 +1,88 @@ +# RFC-0051: Remove the concept of "heap pages" from the client + +| | | +| --------------- | ------------------------------------------------------------------------------------------- | +| **Start Date** | 2023-11-24 | +| **Description** | Remove the concept of heap pages from the client and move it to the runtime. | +| **Authors** | Pierre Krieger | + +## Summary + +Rather that enforce a limit to the total memory consumption on the client side by loading the value at `:heap_pages`, enforce that limit on the runtime side by keeping track. + +## Motivation + +From the early days of Substrate up until recently, the runtime was present in two forms: the wasm runtime (wasm bytecode passed through an interpreter) and the native runtime (native code directly run by the client). + +Since the wasm runtime has a lower amount of memory available (4GiB maximum) compared to the native runtime, and in order to ensure sure that the wasm and native runtimes always produce the same outcome, it was necessary to clamp the amount of memory available to both runtimes to the same value. + +In order to achieve this, a special storage key (a "well-known" key) `:heappages` was introduced and represents the number of "wasm pages" (one page equals 64kiB) of memory that are available to the memory allocator of the runtime. If this storage key is absent, it defaults to 2048, which is 128 MiB. + +The native runtime has since then been disappeared, but the concept of "heap pages" still exists. This RFC proposes a simplification to the design of Polkadot by removing the concept of "heap pages" as is currently known, and proposes an alternative. + +## Stakeholders + +Client implementers. + +## Explanation + +This RFC proposes the following changes to the client: + +- The client no longer considers `:heappages` as special. +- The memory allocator of the runtime is no longer bounded by the value of `:heappages`. + +With these changes, the memory available to the runtime is now only bounded by the available memory space (4 GiB), and optionally by the maximum amount of memory specified in the Wasm binary (see https://webassembly.github.io/spec/core/bikeshed/#memories%E2%91%A0). In Rust, the latter can be controlled during compilation with the flag `-Clink-arg=--max-memory=...`. + +This RFC proposes three alternative paths: + +- Path A: add back the same memory limit to the runtime, like so: + + - At initialization, the runtime loads the value of `:heappages` from the storage (using `ext_storage_get` or similar), and sets a global variable to the decoded value. + - The runtime tracks the total amount of memory that it has allocated using its instance of `#[global_allocator]` (https://github.com/paritytech/polkadot-sdk/blob/e3242d2c1e2018395c218357046cc88caaed78f3/substrate/primitives/io/src/lib.rs#L1748-L1762). This tracking should also be added for the host functions that perform allocations. + - If an allocation is attempted that would go over the value in the global variable, the `unreachable` opcode is called. + +- Path B: defined the memory limit using the `-Clink-arg=--max-memory=...` flag. + +- Path C: don't add anything to the runtime. This is effectively the same as setting the memory limit to ~4 GiB (compared to the current limit of 128 MiB). This solution is viable only because we're compiling for 32bits wasm rather than for example 64bits wasm. If we ever compile for 64bits wasm, this would need to be revisited. + +Furthermore, since the client-side change is strictly more tolerant than before, we can performance the change immediately without having to worry about backwards compatibility. + +The author of this RFC suggests either option C or B. + +## Drawbacks + +In case of path A, there is one situation where the behaviour pre-RFC is not equivalent to the one post-RFC: when a host function that performs an allocation (for example `ext_storage_get`) is called, without this RFC this allocation might fail due to reaching the maximum heap pages, while after this RFC this will always succeed. +This is most likely not a problem, as storage values aren't supposed to be larger than a few megabytes at the very maximum. + +## Testing, Security, and Privacy + +This RFC would reduce the chance of a consensus issue between clients. +The `:heappages` are a rather obscure feature, and it is not clear what happens in some corner cases such as the value being too large (error? clamp?) or malformed. This RFC would completely erase these questions. + +## Performance, Ergonomics, and Compatibility + +### Performance + +In case of path A, it is unclear how performances would be affected. Path A consists in moving client-side operations to the runtime without changing these operations, and as such performance differences are expected to be minimal. Overall, we're talking about one addition/subtraction per malloc and per free, so this is more than likely completely negligible. + +In case of path B and C, the performance gain would be a net positive, as this RFC strictly removes things. + +### Ergonomics + +This RFC would isolate the client and runtime more from each other, making it a bit easier to reason about the client or the runtime in isolation. + +### Compatibility + +Not a breaking change. RFC can be applied immediately without any transition period. + +## Prior Art and References + +None. + +## Unresolved Questions + +None. + +## Future Directions and Related Material + +This RFC follows the same path as https://github.com/polkadot-fellows/RFCs/pull/4 by scoping everything related to memory allocations to the runtime. From 15a2c81c741eb7e56f2fd563d614cb12457260e8 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 24 Nov 2023 14:01:33 +0100 Subject: [PATCH 2/9] Fix RFC number --- text/{0051-remove-heap-pages.md => 0054-remove-heap-pages.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename text/{0051-remove-heap-pages.md => 0054-remove-heap-pages.md} (98%) diff --git a/text/0051-remove-heap-pages.md b/text/0054-remove-heap-pages.md similarity index 98% rename from text/0051-remove-heap-pages.md rename to text/0054-remove-heap-pages.md index db33a7789..e041d25fa 100644 --- a/text/0051-remove-heap-pages.md +++ b/text/0054-remove-heap-pages.md @@ -1,4 +1,4 @@ -# RFC-0051: Remove the concept of "heap pages" from the client +# RFC-0054: Remove the concept of "heap pages" from the client | | | | --------------- | ------------------------------------------------------------------------------------------- | From 24e88a73b2a56b3ac45c930fa2a63c08b2bf93a2 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 24 Nov 2023 14:03:15 +0100 Subject: [PATCH 3/9] Fix sentence --- text/0054-remove-heap-pages.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0054-remove-heap-pages.md b/text/0054-remove-heap-pages.md index e041d25fa..6b546b510 100644 --- a/text/0054-remove-heap-pages.md +++ b/text/0054-remove-heap-pages.md @@ -8,7 +8,7 @@ ## Summary -Rather that enforce a limit to the total memory consumption on the client side by loading the value at `:heap_pages`, enforce that limit on the runtime side by keeping track. +Rather that enforce a limit to the total memory consumption on the client side by loading the value at `:heap_pages`, enforce that limit on the runtime side. ## Motivation From 4f05c77f2495f5f0d376c4fd57ef15156fbd5362 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 24 Nov 2023 14:11:11 +0100 Subject: [PATCH 4/9] Clarify why there are multiple paths --- text/0054-remove-heap-pages.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0054-remove-heap-pages.md b/text/0054-remove-heap-pages.md index 6b546b510..2a8378bcd 100644 --- a/text/0054-remove-heap-pages.md +++ b/text/0054-remove-heap-pages.md @@ -33,7 +33,7 @@ This RFC proposes the following changes to the client: With these changes, the memory available to the runtime is now only bounded by the available memory space (4 GiB), and optionally by the maximum amount of memory specified in the Wasm binary (see https://webassembly.github.io/spec/core/bikeshed/#memories%E2%91%A0). In Rust, the latter can be controlled during compilation with the flag `-Clink-arg=--max-memory=...`. -This RFC proposes three alternative paths: +This RFC proposes three alternative paths (different chains might choose to follow different paths): - Path A: add back the same memory limit to the runtime, like so: @@ -47,7 +47,7 @@ This RFC proposes three alternative paths: Furthermore, since the client-side change is strictly more tolerant than before, we can performance the change immediately without having to worry about backwards compatibility. -The author of this RFC suggests either option C or B. +Each parachain can choose the option that they prefer, but the author of this RFC strongly suggests either option C or B. ## Drawbacks From 53f1b30884408884ef60e3ac84d74ceb69edf6e6 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 24 Nov 2023 14:12:21 +0100 Subject: [PATCH 5/9] Small tweaks --- text/0054-remove-heap-pages.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/text/0054-remove-heap-pages.md b/text/0054-remove-heap-pages.md index 2a8378bcd..042258cd2 100644 --- a/text/0054-remove-heap-pages.md +++ b/text/0054-remove-heap-pages.md @@ -33,20 +33,20 @@ This RFC proposes the following changes to the client: With these changes, the memory available to the runtime is now only bounded by the available memory space (4 GiB), and optionally by the maximum amount of memory specified in the Wasm binary (see https://webassembly.github.io/spec/core/bikeshed/#memories%E2%91%A0). In Rust, the latter can be controlled during compilation with the flag `-Clink-arg=--max-memory=...`. +Since the client-side change is strictly more tolerant than before, we can performance the change immediately without having to worry about backwards compatibility. + This RFC proposes three alternative paths (different chains might choose to follow different paths): - Path A: add back the same memory limit to the runtime, like so: - At initialization, the runtime loads the value of `:heappages` from the storage (using `ext_storage_get` or similar), and sets a global variable to the decoded value. - - The runtime tracks the total amount of memory that it has allocated using its instance of `#[global_allocator]` (https://github.com/paritytech/polkadot-sdk/blob/e3242d2c1e2018395c218357046cc88caaed78f3/substrate/primitives/io/src/lib.rs#L1748-L1762). This tracking should also be added for the host functions that perform allocations. - - If an allocation is attempted that would go over the value in the global variable, the `unreachable` opcode is called. + - The runtime tracks the total amount of memory that it has allocated using its instance of `#[global_allocator]` (https://github.com/paritytech/polkadot-sdk/blob/e3242d2c1e2018395c218357046cc88caaed78f3/substrate/primitives/io/src/lib.rs#L1748-L1762). This tracking should also be added around the host functions that perform allocations. + - If an allocation is attempted that would go over the value in the global variable, the memory allocation fails. -- Path B: defined the memory limit using the `-Clink-arg=--max-memory=...` flag. +- Path B: define the memory limit using the `-Clink-arg=--max-memory=...` flag. - Path C: don't add anything to the runtime. This is effectively the same as setting the memory limit to ~4 GiB (compared to the current limit of 128 MiB). This solution is viable only because we're compiling for 32bits wasm rather than for example 64bits wasm. If we ever compile for 64bits wasm, this would need to be revisited. -Furthermore, since the client-side change is strictly more tolerant than before, we can performance the change immediately without having to worry about backwards compatibility. - Each parachain can choose the option that they prefer, but the author of this RFC strongly suggests either option C or B. ## Drawbacks From 1fc91d5e889abcd67d09f88136162aa4b5c662cf Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 24 Nov 2023 14:26:51 +0100 Subject: [PATCH 6/9] Add drawback about bricking chain --- text/0054-remove-heap-pages.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/text/0054-remove-heap-pages.md b/text/0054-remove-heap-pages.md index 042258cd2..e2a233dfc 100644 --- a/text/0054-remove-heap-pages.md +++ b/text/0054-remove-heap-pages.md @@ -54,6 +54,9 @@ Each parachain can choose the option that they prefer, but the author of this RF In case of path A, there is one situation where the behaviour pre-RFC is not equivalent to the one post-RFC: when a host function that performs an allocation (for example `ext_storage_get`) is called, without this RFC this allocation might fail due to reaching the maximum heap pages, while after this RFC this will always succeed. This is most likely not a problem, as storage values aren't supposed to be larger than a few megabytes at the very maximum. +In the unfortunate event where the runtime runs out of memory, path B would make it more difficult to relax the memory limit, as we would need to re-upload the entire Wasm, compared to updating only `:heappages` in path A or before this RFC. +In the case where the runtime runs out of memory only in the specific event where the Wasm runtime is modified, this could brick the chain. However, this situation is no different than the thousands of other ways that a bug in the runtime can brick a chain, and there's no reason to be particularily worried about this situation in particular. + ## Testing, Security, and Privacy This RFC would reduce the chance of a consensus issue between clients. From 85e37369992e1de73530b9c6f2eda08bbffbca51 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 24 Nov 2023 14:43:50 +0100 Subject: [PATCH 7/9] More small tweaks --- text/0054-remove-heap-pages.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/text/0054-remove-heap-pages.md b/text/0054-remove-heap-pages.md index e2a233dfc..2141a0e83 100644 --- a/text/0054-remove-heap-pages.md +++ b/text/0054-remove-heap-pages.md @@ -8,21 +8,21 @@ ## Summary -Rather that enforce a limit to the total memory consumption on the client side by loading the value at `:heap_pages`, enforce that limit on the runtime side. +Rather that enforce a limit to the total memory consumption on the client side by loading the value at `:heappages`, enforce that limit on the runtime side. ## Motivation From the early days of Substrate up until recently, the runtime was present in two forms: the wasm runtime (wasm bytecode passed through an interpreter) and the native runtime (native code directly run by the client). -Since the wasm runtime has a lower amount of memory available (4GiB maximum) compared to the native runtime, and in order to ensure sure that the wasm and native runtimes always produce the same outcome, it was necessary to clamp the amount of memory available to both runtimes to the same value. +Since the wasm runtime has a lower amount of available memory (4 GiB maximum) compared to the native runtime, and in order to ensure sure that the wasm and native runtimes always produce the same outcome, it was necessary to clamp the amount of memory available to both runtimes to the same value. -In order to achieve this, a special storage key (a "well-known" key) `:heappages` was introduced and represents the number of "wasm pages" (one page equals 64kiB) of memory that are available to the memory allocator of the runtime. If this storage key is absent, it defaults to 2048, which is 128 MiB. +In order to achieve this, a special storage key (a "well-known" key) `:heappages` was introduced and represents the number of "wasm pages" (one page equals 64kiB) of memory that are available to the memory allocator of the runtimes. If this storage key is absent, it defaults to 2048, which is 128 MiB. -The native runtime has since then been disappeared, but the concept of "heap pages" still exists. This RFC proposes a simplification to the design of Polkadot by removing the concept of "heap pages" as is currently known, and proposes an alternative. +The native runtime has since then been disappeared, but the concept of "heap pages" still exists. This RFC proposes a simplification to the design of Polkadot by removing the concept of "heap pages" as is currently known, and proposes alternative ways to achieve the goal of limiting the amount of memory available. ## Stakeholders -Client implementers. +Client implementers and low-level runtime developers. ## Explanation @@ -45,7 +45,7 @@ This RFC proposes three alternative paths (different chains might choose to foll - Path B: define the memory limit using the `-Clink-arg=--max-memory=...` flag. -- Path C: don't add anything to the runtime. This is effectively the same as setting the memory limit to ~4 GiB (compared to the current limit of 128 MiB). This solution is viable only because we're compiling for 32bits wasm rather than for example 64bits wasm. If we ever compile for 64bits wasm, this would need to be revisited. +- Path C: don't add anything to the runtime. This is effectively the same as setting the memory limit to ~4 GiB (compared to the current default limit of 128 MiB). This solution is viable only because we're compiling for 32bits wasm rather than for example 64bits wasm. If we ever compile for 64bits wasm, this would need to be revisited. Each parachain can choose the option that they prefer, but the author of this RFC strongly suggests either option C or B. From 8aebf0e6bb7c55f84f1c577da1819e32dfd62871 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Sun, 26 Nov 2023 09:42:07 +0100 Subject: [PATCH 8/9] Update text/0054-remove-heap-pages.md Co-authored-by: Oliver Tale-Yazdi --- text/0054-remove-heap-pages.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0054-remove-heap-pages.md b/text/0054-remove-heap-pages.md index 2141a0e83..02c24e325 100644 --- a/text/0054-remove-heap-pages.md +++ b/text/0054-remove-heap-pages.md @@ -8,7 +8,7 @@ ## Summary -Rather that enforce a limit to the total memory consumption on the client side by loading the value at `:heappages`, enforce that limit on the runtime side. +Rather than enforce a limit to the total memory consumption on the client side by loading the value at `:heappages`, enforce that limit on the runtime side. ## Motivation From cc0699cbd2f340667898641787461c2cf726e7b6 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Mon, 27 Nov 2023 14:25:52 +0100 Subject: [PATCH 9/9] Typo and clarifications about backcompat --- text/0054-remove-heap-pages.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0054-remove-heap-pages.md b/text/0054-remove-heap-pages.md index 02c24e325..fec9cc8c2 100644 --- a/text/0054-remove-heap-pages.md +++ b/text/0054-remove-heap-pages.md @@ -33,7 +33,7 @@ This RFC proposes the following changes to the client: With these changes, the memory available to the runtime is now only bounded by the available memory space (4 GiB), and optionally by the maximum amount of memory specified in the Wasm binary (see https://webassembly.github.io/spec/core/bikeshed/#memories%E2%91%A0). In Rust, the latter can be controlled during compilation with the flag `-Clink-arg=--max-memory=...`. -Since the client-side change is strictly more tolerant than before, we can performance the change immediately without having to worry about backwards compatibility. +Since the client-side change is strictly more tolerant than before, we can perform the change immediately after the runtime has been updated, and without having to worry about backwards compatibility. This RFC proposes three alternative paths (different chains might choose to follow different paths): @@ -76,7 +76,7 @@ This RFC would isolate the client and runtime more from each other, making it a ### Compatibility -Not a breaking change. RFC can be applied immediately without any transition period. +Not a breaking change. The runtime-side changes can be applied immediately (without even having to wait for changes in the client), then as soon as the runtime is updated, the client can be updated without any transition period. One can even consider updating the client before the runtime, as it corresponds to path C. ## Prior Art and References