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

Introduce crumbles - keeping track of dirt in userspace #255

Merged
merged 15 commits into from
Aug 28, 2023
Merged

Conversation

ureeves
Copy link
Member

@ureeves ureeves commented Aug 16, 2023

crumbles is intended to solve the problem of mapping WASM memories onto virtual address space.

It is built to support tracking dirty pages and creating memory snapshots to which it is possible to revert to. This is required if we want to remove re-execution in piecrust, as well as allowing for the removal of diffing in the storage process of state commits.

Since it uses system calls that only exist on *nix systems it can only build on Linux and MacOS. The tracking of dirty pages is done slightly differently on Linux and MacOS, and it might be useful to test it actively for different combinations of platform and architecture. It is important to note that using this would not be a regression for piecrust since it currently only builds on *nix as well.

See also: #253
See also: #254

@ureeves ureeves added team:Core Low Level Core Development Team (Rust) type:enhancement Issues concerning code or feature improvement (performance, refactoring, etc) labels Aug 16, 2023
Copy link
Contributor

@fed-franz fed-franz left a comment

Choose a reason for hiding this comment

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

LGTM!

Nice lib and love the name :)

Couple of nits in the comments.

Also: since you define MEM_SIZE and PAGE_SIZE, it might be worth using those in the description, instead of assuming is 4Gib/64Kib. This would turn useful if you plan to release the lib with those value as input parameters.

crumbles/src/lib.rs Outdated Show resolved Hide resolved
crumbles/src/lib.rs Outdated Show resolved Hide resolved
crumbles/src/lib.rs Outdated Show resolved Hide resolved
crumbles/src/lib.rs Outdated Show resolved Hide resolved
crumbles/src/lib.rs Outdated Show resolved Hide resolved
@ureeves
Copy link
Member Author

ureeves commented Aug 16, 2023

LGTM!

Nice lib and love the name :)

Couple of nits in the comments.

Also: since you define MEM_SIZE and PAGE_SIZE, it might be worth using those in the description, instead of assuming is 4Gib/64Kib. This would turn useful if you plan to release the lib with those value as input parameters.

I haven't made those public, so referring to them in the docs would be wrong. I'll make them public, since it would be a breaking change to modify them anyway.

This means we also remove references to the underlying number in the
documentation, making it easily changeable in the future.
@ureeves ureeves requested a review from Daksh14 August 17, 2023 09:19
Copy link
Member

@moCello moCello left a comment

Choose a reason for hiding this comment

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

LGTM, just some tinitiny suggestions but nothing major.
Is rustfmt.toml supposed to be deleted though?

crumbles/src/lib.rs Outdated Show resolved Hide resolved
crumbles/src/lib.rs Outdated Show resolved Hide resolved
crumbles/src/lib.rs Outdated Show resolved Hide resolved
crumbles/src/lib.rs Show resolved Hide resolved
@ureeves ureeves merged commit 0d18ab8 into main Aug 28, 2023
4 checks passed
@ureeves ureeves deleted the crumbles branch August 28, 2023 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:Core Low Level Core Development Team (Rust) type:enhancement Issues concerning code or feature improvement (performance, refactoring, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants