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

Make sleep work with isolation enabled #2506

Merged
merged 7 commits into from
Sep 13, 2022
Merged

Conversation

pvdrz
Copy link
Contributor

@pvdrz pvdrz commented Aug 23, 2022

Implement a virtual monotone clock that can be used to track time while isolation is enabled. This virtual clock keeps an internal nanoseconds counter that will be increased by a fixed amount at the end of every basic block.

When a process sleeps, this clock will return immediately and increase the counter by the interval the process was supposed to sleep. Making miri execution faster than native code :trollface:.

cc @RalfJung @saethlin @JakobDegen

src/shims/time.rs Outdated Show resolved Hide resolved
src/concurrency/thread.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
src/concurrency/thread.rs Outdated Show resolved Hide resolved
@pvdrz pvdrz force-pushed the a-really-bad-clock branch from 0215536 to 5ba4ba5 Compare August 26, 2022 02:16
@bors
Copy link
Contributor

bors commented Aug 26, 2022

☔ The latest upstream changes (presumably #2363) made this pull request unmergeable. Please resolve the merge conflicts.

@pvdrz pvdrz force-pushed the a-really-bad-clock branch from 5ba4ba5 to 3b69f69 Compare August 26, 2022 16:55
src/concurrency/thread.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
src/concurrency/thread.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Aug 31, 2022

☔ The latest upstream changes (presumably #2524) made this pull request unmergeable. Please resolve the merge conflicts.

@pvdrz pvdrz force-pushed the a-really-bad-clock branch from 0e2ff88 to d707994 Compare August 31, 2022 21:38
src/shims/time.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member

RalfJung commented Sep 1, 2022

I did some tweaking to the API to make it match the one from the standard library more closely, which overall makes this a much smaller conceptual change for the rest of Miri. Please take a look and let me know what you think. :)

If you like it, please squash the commits together a little -- this is ready to go, as far as I am concerned.

@saethlin saethlin added the S-waiting-on-author Status: Waiting for the PR author to address review comments label Sep 4, 2022
Copy link
Member

@saethlin saethlin left a comment

Choose a reason for hiding this comment

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

I just have nits. This is a much smaller and more maintainable change than I somehow expected!

src/clock.rs Outdated Show resolved Hide resolved
src/clock.rs Outdated Show resolved Hide resolved
src/shims/time.rs Outdated Show resolved Hide resolved
tests/pass/shims/time-with-isolation.rs Outdated Show resolved Hide resolved
src/clock.rs Outdated Show resolved Hide resolved
@pvdrz pvdrz requested a review from saethlin September 12, 2022 21:27
@saethlin
Copy link
Member

👍 Squash the commits down some then I'll approve this. (also if you want a nice description of this to live in the commit history you could write a PR description)

@pvdrz pvdrz force-pushed the a-really-bad-clock branch from 21e3a76 to c834637 Compare September 13, 2022 20:16
@saethlin
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 13, 2022

📌 Commit c834637 has been approved by saethlin

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Sep 13, 2022

⌛ Testing commit c834637 with merge 2f1fa12...

@saethlin saethlin dismissed their stale review September 13, 2022 21:20

addressed

@bors
Copy link
Contributor

bors commented Sep 13, 2022

☀️ Test successful - checks-actions
Approved by: saethlin
Pushing 2f1fa12 to master...

@bors bors merged commit 2f1fa12 into rust-lang:master Sep 13, 2022
@pvdrz pvdrz deleted the a-really-bad-clock branch September 14, 2022 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: Waiting for the PR author to address review comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants