Add turmoil::sim_elapsed for retrieving total simulation virtual time
#164
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mcches
approved these changes
Jan 10, 2024
examples/grpc/src/main.rs
Outdated
| fn format_time(&self, w: &mut tracing_subscriber::fmt::format::Writer<'_>) -> std::fmt::Result { | ||
| // The debug implementation of Duration gives reasonably formatted | ||
| // durations (e.g. `610ms`, `2.62s`). | ||
| write!(w, "{:?}", turmoil::sim_elapsed().unwrap_or_default()) |
There was a problem hiding this comment.
I think it is valuable to see real and sim time in the trace. Up to you on whether you think that is useful in the example.
| /// virtual time. | ||
| /// | ||
| /// Must be called from within a Turmoil simulation. | ||
| pub fn elapsed() -> Duration { |
There was a problem hiding this comment.
While the docs are the same, this is definitely more readable in the source code.
added 2 commits
January 10, 2024 09:33
Previously, we only exposed `turmoil::elapsed` which returned the elapsed virtual time for the currently executing host. If you step your simulation for some duration before registering a host then the host's elapsed time will be less than the overall sim elapsed time. We didn't want to make a breaking API change for this, so opted to add a new function (`turmoil::sim_elapsed`). This function returns an `Option` so that the caller can gracefully handle failure (`turmoil::elapsed` will crash if called outside of an executing host, for example). This makes it easier to use `sim_elapsed` for things like logging elapsed time via the std tracing subscriber (there's an example of how to do this in the `grpc` example). One sharp edge I ran into is that we sometimes do tracing in this crate itself (e.g. in `Tcp::bind`) which happens while we're holding a mutable borrow of the thread-local `RefCell<World>`. Since the tracing event causes us to call `tracing::sim_elapsed`, we have an attempted double mutable borrow. In these cases I just made `sim_elapsed` return `None`. It's a bit of a wart but I think it's okay. We also return `None` when `sim_elapsed` is called when there's no executing host, so I think the API returning `Option` feels right. We can internally fix the double borrow issue in future without a breaking change.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, we only exposed
turmoil::elapsedwhich returned the elapsed virtual time for the currently executing host. If you step your simulation for some duration before registering a host then the host's elapsed time will be less than the overall sim elapsed time.We didn't want to make a breaking API change for this, so opted to add a new function (
turmoil::sim_elapsed). This function returns anOptionso that the caller can gracefully handle failure (turmoil::elapsedwill crash if called outside of an executing host, for example). This makes it easier to usesim_elapsedfor things like logging elapsed time via the std tracing subscriber (there's an example of how to do this in thegrpcexample).One sharp edge I ran into is that we sometimes do tracing in this crate itself (e.g. in
Tcp::bind) which happens while we're holding a mutable borrow of the thread-localRefCell<World>. Since the tracing event causes us to calltracing::sim_elapsed, we have an attempted double mutable borrow. In these cases I just madesim_elapsedreturnNone. It's a bit of a wart but I think it's okay. We also returnNonewhensim_elapsedis called when there's no executing host, so I think the API returningOptionfeels right. We can internally fix the double borrow issue in future without a breaking change.