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

Improve module accessors performance #7

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/actions/ci-cache/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ runs:
steps:
# It would be nice to factorize path and key.
- if: inputs.mode == 'restore'
uses: actions/cache/restore@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2
uses: actions/cache/restore@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0
with:
path: |
~/.cargo/bin/
Expand All @@ -23,7 +23,7 @@ runs:
key: ${{ hashFiles('rust-toolchain.toml', 'scripts/wrapper.sh') }}
- if: inputs.mode == 'save'
id: cache
uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2
uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0
with:
path: |
~/.cargo/bin/
Expand Down
9 changes: 1 addition & 8 deletions .github/actions/ci-checks/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,7 @@ runs:
run: ./scripts/publish.sh --dry-run
shell: bash
- if: ${{ contains(fromJSON(inputs.checks), 'markdown') }}
# TODO: Keep only the last command once ubuntu-latest is 24.04.
run: |
set -x
curl -Os http://ftp.de.debian.org/debian/pool/main/r/ruby-mdl/markdownlint_0.13.0-4_all.deb
sudo apt install ruby-kramdown-parser-gfm ruby-mixlib-{cli,config,shellout}
sudo dpkg -i markdownlint_0.13.0-4_all.deb
rm markdownlint_0.13.0-4_all.deb
./scripts/wrapper.sh mdl -g -s markdownlint.rb .
run: ./scripts/wrapper.sh mdl -g -s markdownlint.rb .
shell: bash
- if: ${{ contains(fromJSON(inputs.checks), 'taplo') }}
run: ./scripts/ci-taplo.sh
Expand Down
2 changes: 1 addition & 1 deletion .github/actions/ci-footprint/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ runs:
- run: mv footprint.toml footprint-${{ github.event_name }}.toml
shell: bash
- if: github.event_name == 'push' && inputs.upload == 'true'
uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3
uses: actions/upload-artifact@6f51ac03b9356f520e9adb1b1b7802705f340c2b # v4.5.0
with:
name: footprint
path: footprint-push.toml
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ jobs:
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
- run: ./scripts/artifacts.sh
- uses: actions/attest-build-provenance@ef244123eb79f2f7a7e75d99086184180e6d0018 # v1.4.4
- uses: actions/attest-build-provenance@7668571508540a607bdfd90a87a560489fe372eb # v2.1.0
id: attest
with:
subject-path: 'artifacts/*'
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/scorecard.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ jobs:
results_file: results.sarif
results_format: sarif
publish_results: true
- uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3
- uses: actions/upload-artifact@6f51ac03b9356f520e9adb1b1b7802705f340c2b # v4.5.0
with:
name: SARIF file
path: results.sarif
retention-days: 5
- uses: github/codeql-action/upload-sarif@f09c1c0a94de965c15400f5634aa42fac8fb8f88 # v3.27.5
- uses: github/codeql-action/upload-sarif@48ab28a6f5dbc2a99bf1e0131198dd8f1df78169 # v3.28.0
with:
sarif_file: results.sarif
2 changes: 1 addition & 1 deletion book/src/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# [Introduction](https://google.github.io/wasefire)

This book is a walk through the _Wasefire_ project.
This book is a walkthrough of the _Wasefire_ project.

## Vision

Expand Down
2 changes: 1 addition & 1 deletion book/src/applet/prelude/usb.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ We then wait until the player presses Enter. We can read a single byte from the
{{#include usb.rs:ready}}
```

To generate the next question, we use `rng::fill_bytes()` which fills a buffer with random bytes. We
To generate the next question, we use `rng::bytes()` which returns a slice of random bytes. We
provide a buffer with the length of the current level. For the string to be printable we truncate
the entropy of each byte from 8 to 5 bits and convert it to a `base32` symbol.

Expand Down
7 changes: 3 additions & 4 deletions book/src/applet/prelude/usb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
#![no_std]
wasefire::applet!();

use alloc::format;
use alloc::rc::Rc;
use alloc::string::String;
use alloc::{format, vec};
use core::cell::Cell;
use core::time::Duration;

Expand All @@ -49,13 +49,12 @@ fn main() {

//{ ANCHOR: generate
// Generate a question for this level.
let mut question = vec![0; level];
rng::fill_bytes(&mut question).unwrap();
let mut question = rng::bytes(level).unwrap();
for byte in &mut question {
const BASE32: [u8; 32] = *b"ABCDEFGHIJKLMNOPQRSTUVWXYZ234567";
*byte = BASE32[(*byte & 0x1f) as usize];
}
let mut question = String::from_utf8(question).unwrap();
let mut question = String::from_utf8(question.into()).unwrap();
//} ANCHOR_END: generate

//{ ANCHOR: question
Expand Down
4 changes: 3 additions & 1 deletion crates/api-desc/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Minor

- Improve safety documentation
- Add `Api::wasm_markdown()` for top-level documentation
- Use Rust edition 2024
- Use C-string literals to implement dispatch for native applets
- Implement `bytemuck::Pod` for generated `Params`
Expand Down Expand Up @@ -137,4 +139,4 @@

## 0.1.0

<!-- Increment to skip CHANGELOG.md test: 1 -->
<!-- Increment to skip CHANGELOG.md test: 3 -->
2 changes: 1 addition & 1 deletion crates/api-desc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ internal-api-store = []
internal-api-usb = []

[lints]
clippy.literal-string-with-formatting-args = "allow"
clippy.mod-module-files = "warn"
clippy.unit-arg = "allow"
rust.unreachable-pub = "warn"
rust.unsafe-op-in-unsafe-fn = "warn"
rust.unused-crate-dependencies = "warn"
2 changes: 1 addition & 1 deletion crates/api-desc/crates/update/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ clap = { version = "4.5.4", default-features = false, features = ["derive", "std
wasefire-applet-api-desc = { path = "../.." }

[lints]
clippy.literal-string-with-formatting-args = "allow"
clippy.mod-module-files = "warn"
clippy.unit-arg = "allow"
rust.unreachable-pub = "warn"
rust.unsafe-op-in-unsafe-fn = "warn"
rust.unused-crate-dependencies = "warn"
11 changes: 10 additions & 1 deletion crates/api-desc/crates/update/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ fn main() -> Result<()> {

"#,
)?;
Api::default().wasm(&mut output, flags.lang.into())?;
let api = Api::default();
for line in api.wasm_markdown().lines() {
write!(&mut output, "//")?;
if !line.is_empty() {
write!(&mut output, " ")?;
}
writeln!(&mut output, "{}", line.strip_prefix("##").unwrap_or(line))?;
}
writeln!(&mut output)?;
api.wasm(&mut output, flags.lang.into())?;
Ok(())
}
78 changes: 78 additions & 0 deletions crates/api-desc/src/api.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
### Applet functions

An applet must expose 3 functions to the platform:

- `init: () -> ()` called exactly once before anything else. It cannot call any
platform function except `dp` (aka `debug::println()`).
- `main: () -> ()` called exactly once after `init` returns. It can call all
platform functions.
- `alloc: (size: u32, align: u32) -> (ptr: u32)` may be called during any
platform function after `init` returned. It cannot call any platform function.
A return value of zero indicates an error and will trap the applet. The applet
may assume that `size` is a non-zero multiple of `align` and `align` is either
1, 2, or 4.

### Platform functions

The platform exposes many functions. Each function comes with its own
documentation, which may omit the following general properties.

#### Parameters and result

At WebAssembly level, all functions use `u32` (more precisely `i32`) as
parameters and result (WebAssembly permits multiple results, but platform
functions never return more than one value). However, platform functions will be
documented with more specific types to better convey the intent.

Languages that compile to WebAssembly may use similar specific types when
binding platform functions. Those types should be compatible with `u32` in
WebAssembly.

#### Applet closures

Some functions (usually called `register`) may register a closure. There is
usually an associated function (usually called `unregister`) that unregisters
the registered closure. The register function takes `fn { data: *const void, ...
}` (usually called `handler_func`) and a `*const void` (usually called
`handler_data`) as arguments. The unregister function (if it exists) doesn't
have any particular parameters.

After the closure is registered (the register function is called) and as long as
it stays registered (the unregister function is not called), the platform may
call this closure any time `sw` (aka `scheduling::wait_for_callback()`) is
called. Note that if `main` returns and there are registered closures, the
applet behaves as if `sw` is called indefinitely.

An applet closure may call any platform function unless explicitly documented by
the register function.

#### Reading and writing memory

Some functions take pointers as argument (usually `*const u8` or `*mut u8`).
They either document the expected size or take an associated length argument.

The platform may read and write (only for `*mut T` pointers) the designated
region of the WebAssembly memory during the function call. Otherwise (and by
default), platform functions don't read or write the WebAssembly memory.

#### Allocating memory

Finally, some functions take nested pointers as argument (usually `*mut *mut
u8`). They either return a `usize` or take a `*mut usize` as argument.

Those functions may call `alloc` (at most once per such nested pointer argument)
with a dynamic size but fixed alignment (usually 1). If they do, they will store
the non-null result in the `*mut *mut u8` argument. Regardless of whether they
allocate or not, they store the size in the `*mut usize` argument (if it exists)
or return it (otherwise).

Note that `alloc` will not be called with a size of zero. So if the size is zero
after the function returns, this signifies that `alloc` was not called (and the
`*mut *mut u8` not written) and the output is empty (unless otherwise documented
like in the case of an absence of output, in which case the size may not be
written).

The caller is responsible for managing the allocation after the function returns
and the size is non-zero. To avoid memory leaks, the applet should initialize
the `*mut usize` argument (if it exists) to zero and consider having ownership
of the `*mut *mut u8` pointee if the size is non-zero after the call.
12 changes: 8 additions & 4 deletions crates/api-desc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ impl Api {
}
}

pub fn wasm_markdown(&self) -> &'static str {
include_str!("api.md")
}

pub fn wasm_rust(&self) -> TokenStream {
let items: Vec<_> = self.0.iter().map(|x| x.wasm_rust()).collect();
quote!(#(#items)*)
Expand Down Expand Up @@ -730,7 +734,7 @@ mod tests {
#[test]
fn link_names_are_unique() {
let mut seen = HashSet::new();
let Api(mut todo) = Api::default();
let mut todo = Api::default().0;
while let Some(item) = todo.pop() {
match item {
Item::Enum(_) => (),
Expand All @@ -746,7 +750,7 @@ mod tests {
/// This invariant is assumed by unsafe code.
#[test]
fn enum_values_are_valid() {
let Api(mut todo) = Api::default();
let mut todo = Api::default().0;
while let Some(item) = todo.pop() {
match item {
Item::Enum(Enum { variants, .. }) => {
Expand All @@ -773,7 +777,7 @@ mod tests {
let name = &field.name;
assert!(field.type_.is_param(), "Param {name} of {link:?} is not U32");
}
let Api(mut todo) = Api::default();
let mut todo = Api::default().0;
while let Some(item) = todo.pop() {
match item {
Item::Enum(_) => (),
Expand All @@ -786,7 +790,7 @@ mod tests {

#[test]
fn results_are_valid() {
let Api(mut todo) = Api::default();
let mut todo = Api::default().0;
while let Some(item) = todo.pop() {
match item {
Item::Enum(_) => (),
Expand Down
18 changes: 4 additions & 14 deletions crates/api-desc/src/platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,31 +31,21 @@ pub(crate) fn new() -> Item {
update::new(),
#[cfg(feature = "api-platform")]
item! {
/// Returns the serial of the platform.
/// Reads the serial of the platform.
///
/// Returns the length of the serial in bytes. The serial is not allocated if the
/// length is zero (and the pointer is not written).
/// This is an [allocating function](crate#allocating-memory) returning the length.
fn serial "ps" {
/// Where to write the serial.
///
/// If the returned length is positive, the (inner) pointer will be allocated by the
/// callee and must be freed by the caller. It is thus owned by the caller when the
/// function returns.
ptr: *mut *mut u8,
} -> usize
},
#[cfg(feature = "api-platform")]
item! {
/// Returns the version of the platform.
/// Reads the version of the platform.
///
/// Returns the length of the version in bytes. The version is not allocated if the
/// length is zero (and the pointer is not written).
/// This is an [allocating function](crate#allocating-memory) returning the length.
fn version "pv" {
/// Where to write the version.
///
/// If the returned length is positive, the (inner) pointer will be allocated by the
/// callee and must be freed by the caller. It is thus owned by the caller when the
/// function returns.
ptr: *mut *mut u8,
} -> usize
},
Expand Down
7 changes: 3 additions & 4 deletions crates/api-desc/src/platform/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,11 @@ pub(crate) fn new() -> Item {
item! {
/// Reads the last request, if any.
///
/// Returns whether a request was allocated.
/// Returns whether a request was read.
///
/// This is an [allocating function](crate#allocating-memory).
fn read "ppr" {
/// Where to write the request, if any.
///
/// The (inner) pointer will be allocated by the callee and must be freed by the
/// caller. It is thus owned by the caller when the function returns.
ptr: *mut *mut u8,

/// Where to write the length of the request, if any.
Expand Down
7 changes: 5 additions & 2 deletions crates/api-desc/src/platform/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@ pub(crate) fn new() -> Item {
fn is_supported "pus" {} -> bool
},
item! {
/// Returns the metadata of the platform.
/// Reads the metadata of the platform.
///
/// This typically contains the version and side (A or B) of the running platform.
/// The metadata typically contains the version and side (A or B) of the running
/// platform.
///
/// This is an [allocating function](crate#allocating-memory).
fn metadata "pum" {
/// Where to write the allocated metadata.
ptr: *mut *mut u8,
Expand Down
14 changes: 5 additions & 9 deletions crates/api-desc/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,13 @@ pub(crate) fn new() -> Item {
/// Finds an entry in the store, if any.
///
/// Returns whether an entry was found.
///
/// This is an [allocating function](crate#allocating-memory).
fn find "sf" {
/// Key of the entry to find.
key: usize,

/// Where to write the value of the entry, if found.
///
/// The (inner) pointer will be allocated by the callee and must be freed by the
/// caller. It is thus owned by the caller when the function returns.
ptr: *mut *mut u8,

/// Where to write the length of the value, if found.
Expand All @@ -74,13 +73,10 @@ pub(crate) fn new() -> Item {
item! {
/// Returns the unordered keys of the entries in the store.
///
/// Returns the number of keys, and thus the length of the array. The array is not
/// allocated if the length is zero (and the pointer is not written).
/// This is an [allocating function](crate#allocating-memory) returning the number of
/// keys (thus half the number of allocated bytes).
fn keys "sk" {
/// Where to write the keys as an array of u16, if at least one.
///
/// The (inner) pointer will be allocated by the callee and must be freed by the
/// caller. It is thus owned by the caller when the function returns.
/// Where to write the keys as an array of u16.
ptr: *mut *mut u8,
} -> usize
},
Expand Down
Loading
Loading