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

⏱️ At a hostcall to get the amount of vcpu time that has passed in the guest, in milliseconds #412

Merged
merged 13 commits into from
Aug 16, 2024

Conversation

acw
Copy link
Contributor

@acw acw commented Jul 29, 2024

This is a proposed extension, to help customers diagnose where their code is spending time.

One explicit thing to mention is that while this hostcall limits itself to vcpu time (and thus does not count any time spent blocking on IO, for example), it is based on a time passing rather than cycles, WASM instruction count, or any other timer that is independent of the underlying processor(s) running the code. Thus, the values found should only really be compared to other values captured in the exact same run.

@@ -268,6 +268,24 @@ pub mod fastly_abi {
}
}

pub mod fastly_vcpu {
Copy link
Contributor

Choose a reason for hiding this comment

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

drive-by feedback: might be worth thinking whether to use a more generic module name where we can add other bits of introspection. E.g.: fastly_introspect or fastly_meta or similar...

Copy link
Contributor

Choose a reason for hiding this comment

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

(I can imagine a lot of additional introspection we might want to add over time, on other aspects of the guest behavior.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with fastly_compute_runtime, which I like as a general future module for getting or setting or otherwise looking at the behavior of a session itself.

@acw acw marked this pull request as ready for review July 31, 2024 23:32
lib/src/session.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@ulyssa ulyssa left a comment

Choose a reason for hiding this comment

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

This looks good to me! ✨ Just the question about the adapter wasm.

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you rerun make adapter after merging main so that this includes both the vcpu hostcall and the inspect hostcall?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup! Rerunning it now shows me no changes with this PR.

@acw acw merged commit 3192cbf into main Aug 16, 2024
15 checks passed
@acw acw deleted the awick/get-vcpu-ms branch August 16, 2024 21:05
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.

4 participants