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

[crates] Add page-cache crate #268

Merged
merged 1 commit into from
Jul 22, 2022
Merged

Conversation

lucassong-mh
Copy link
Contributor

Add page-cache crate #267.

Copy link
Contributor

@tatetian tatetian left a comment

Choose a reason for hiding this comment

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

Good job. The PR looks good to me in general. With that said, there are plenty of little things that we can improve to make the code even better.

src/libos/crates/page-cache/src/lib.rs Outdated Show resolved Hide resolved
src/libos/crates/page-cache/src/page.rs Outdated Show resolved Hide resolved
src/libos/crates/page-cache/src/page_alloc.rs Outdated Show resolved Hide resolved
src/libos/crates/page-cache/src/page_alloc.rs Outdated Show resolved Hide resolved
src/libos/crates/page-cache/src/page_alloc.rs Outdated Show resolved Hide resolved
src/libos/crates/page-cache/src/cached_disk.rs Outdated Show resolved Hide resolved
src/libos/crates/page-cache/src/cached_disk.rs Outdated Show resolved Hide resolved
src/libos/crates/page-cache/src/cached_disk.rs Outdated Show resolved Hide resolved
src/libos/crates/page-cache/src/cached_disk.rs Outdated Show resolved Hide resolved
src/libos/crates/page-cache/src/cached_disk.rs Outdated Show resolved Hide resolved
@lucassong-mh lucassong-mh force-pushed the dev-page_cache branch 2 times, most recently from c0cf83f to 0ceed1b Compare July 14, 2022 14:38
@tatetian
Copy link
Contributor

Could you make my life as a reviewer a bit easier by clicking the "Resolve conversation" button for the issues that you have fixed? Comparing between the old and new versions one location at a time is very time consuming.

For issues that you have different opinions, I would like to see you replies.

In Liqing's PR of async file systems, a crate called lru-rs is introduced (see deps/lru-rs in this commit). Do you think the LRU cache provided by lru-rs can satisfy your needs? If so, I think we should avoid reinventing the wheels and replace util/lru_cache with lru-rs.

@lucassong-mh lucassong-mh force-pushed the dev-page_cache branch 3 times, most recently from 7cae157 to d9d1612 Compare July 19, 2022 03:11
Copy link
Contributor

@tatetian tatetian left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the contribution!

@tatetian tatetian merged commit 9c55d90 into occlum:master Jul 22, 2022
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.

2 participants