-
Notifications
You must be signed in to change notification settings - Fork 316
(Proof-of-Concept/Draft) Pre-parsing feature #1636
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
base: main
Are you sure you want to change the base?
Conversation
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.
I already started reviewing this PR before I noticed that it is just a "draft". So sorry for that. However, I do not want to hold back what I have already reviewed and commented in case it helps you.
const REVCOMP_INPUT: &[u8] = include_bytes!("rust/cases/reverse_complement/input.txt"); | ||
const REVCOMP_OUTPUT: &[u8] = include_bytes!("rust/cases/reverse_complement/output.txt"); | ||
const REVCOMP_INPUT: &[u8] = &[42]; //include_bytes!("rust/cases/reverse_complement/input.txt"); | ||
const REVCOMP_OUTPUT: &[u8] = &[42]; //include_bytes!("rust/cases/reverse_complement/output.txt"); |
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.
Why have you effectively disabled this benchmark?
serde = { version = "1.0", default-features = false, features = [ | ||
"derive", | ||
"alloc", | ||
], optional = true } | ||
postcard = { workspace = true, default-features = false, optional = true, features = [ | ||
"alloc", | ||
] } |
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.
Please do not re-format code that is not associated to your PR diff.
criterion_group, | ||
criterion_main, | ||
measurement::WallTime, | ||
Bencher, | ||
BenchmarkGroup, | ||
Criterion, | ||
criterion_group, criterion_main, measurement::WallTime, Bencher, BenchmarkGroup, Criterion, |
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.
Please stick to Wasmi's formatting rules.
/// Initializes the [`EngineFunc`] for lazy translation. | ||
/// | ||
/// # Panics | ||
/// | ||
/// - If `func` is an invalid [`EngineFunc`] reference for this [`CodeMap`]. | ||
/// - If `func` refers to an already initialized [`EngineFunc`]. | ||
pub fn init_func_as_uncompiled( | ||
&self, | ||
func: EngineFunc, | ||
func_idx: FuncIdx, | ||
bytes: &[u8], | ||
module: &ModuleHeader, | ||
func_to_validate: Option<FuncToValidate<ValidatorResources>>, | ||
) { | ||
let mut funcs = self.funcs.lock(); | ||
let Some(func) = funcs.get_mut(func) else { | ||
panic!("encountered invalid internal function: {func:?}") | ||
}; | ||
func.init_uncompiled(UncompiledFuncEntity::new( | ||
func_idx, | ||
bytes, | ||
module.clone(), | ||
func_to_validate, | ||
)); | ||
} | ||
|
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.
Please don't unnecessarily move functions around that have no effect on the PRs diff. This just makes it harder to review and blow-up the PR's diff.
crates/wasmi/src/engine/config.rs
Outdated
/// Sets the [`StackLimits`] for the [`Config`]. | ||
pub fn set_stack_limits(&mut self, stack_limits: StackLimits) -> &mut Self { | ||
self.stack_limits = stack_limits; | ||
self | ||
} | ||
|
||
/// Returns the [`StackLimits`] of the [`Config`]. | ||
pub(super) fn stack_limits(&self) -> StackLimits { | ||
self.stack_limits | ||
} | ||
|
||
/// Sets the maximum amount of cached stacks for reuse for the [`Config`]. | ||
/// | ||
/// # Note | ||
/// | ||
/// Defaults to 2. | ||
pub fn set_cached_stacks(&mut self, amount: usize) -> &mut Self { | ||
self.cached_stacks = amount; | ||
self | ||
} | ||
|
||
/// Returns the maximum amount of cached stacks for reuse of the [`Config`]. | ||
pub(super) fn cached_stacks(&self) -> usize { | ||
self.cached_stacks | ||
} | ||
|
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.
Please rebase your work onto main
. These methods already do not exist on main
.
- new module in the wasmi crate - tests for (roundtrip) (de)serialization of Wasm modules
- introduce features to enable operation modes: - default (current way) - parser + serialization - deserialization
1957af0
to
fae56e1
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1636 +/- ##
==========================================
+ Coverage 70.39% 71.01% +0.61%
==========================================
Files 179 187 +8
Lines 15344 15799 +455
==========================================
+ Hits 10802 11220 +418
- Misses 4542 4579 +37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Proof of Concept: Module Preparsing for Resource-Constrained Targets
This is a proof-of-concept implementation intended to demonstrate the concept and gather feedback. It is NOT intended for production use and contains several incomplete and unclean aspects that would need to be addressed in a proper implementation.
I'm happy to work with the Wasmi maintainers to implement this feature properly, starting from scratch if needed.
Overview
This PR demonstrates a module preparsing feature that enables Wasmi to run on resource-constrained embedded systems by allowing modules to be parsed and validated on development machines, then serialized to a compact format for lightweight runtime loading.
Problem Statement (also described in #1637 )
When deploying Wasmi on resource-constrained targets (e.g., nRF53 with ~1MB total memory), the current implementation consumes significant memory (~480 KiB when deployed with Embassy). This limits the ability to run complex WebAssembly workloads on embedded devices.
Proposed Solution
The feature adds module serialization and deserialization capabilities, allowing:
Implementation Details
New Features Added
serialization
feature for creating serialized modulesdeserialization
feature for lightweight runtime buildsparser
feature-gates the wasmi logic for parsing WebAssembly modules (compiling without this feature provides slim builds by excluding the parsing/validation/translation machinery)New Modules
preparsed/
: Main module for serialization/deserializationpreparsed/serialized_module/
: Data structures for serialized formatpreparsed/serialization/
: Module serialization logicpreparsed/deserialization/
: Module deserialization logicpreparsed/error.rs
: Comprehensive error handlingSerialization Format
Performance Impact
Based on testing with nRF53 + Embassy + BLE stack:
What's Incomplete/Unclean
This proof-of-concept has several areas that need improvement:
1. Module Internals Visibility
pub(crate)
visibility modifiers may be too permissive2. Error Handling
3. Feature Completeness
4. Architecture Integration
Example Repository
I've created an example repository demonstrating the usage and performance benefits: Preparsing Example
The repository shows:
Next Steps
If this approach is approved:
Questions for Maintainers