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

Add support for the get_vcpu_ms hostcall #129

Merged
merged 17 commits into from
Sep 3, 2024
Merged

Conversation

acw
Copy link
Contributor

@acw acw commented Aug 15, 2024

This adds support for a hostcall that fetches the amount of time that the guest has spent on the VCPU, but does not include any time the guest has spent idle. The Viceroy implementation is forthcoming.

The raw hostcall provides this value in milliseconds, but the little reading on time in Go suggested that translating this to a
Duration would be more natural. I can revisit this decision, or obviously folks can reach in and get the raw millisecond value from the internal library.

The test case included makes sure that we don't count sleeping time. It may be worth adding an additional test to verify that we do count active time.

Copy link
Member

@joeshaw joeshaw left a comment

Choose a reason for hiding this comment

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

The code itself looks good, so my comments below are mostly build and idiom related.

compute_runtime/doc.go Outdated Show resolved Hide resolved
compute_runtime/compute_runtime.go Outdated Show resolved Hide resolved
compute_runtime/compute_runtime.go Outdated Show resolved Hide resolved
compute/compute.go Outdated Show resolved Hide resolved
integration_tests/compute_runtime/main_test.go Outdated Show resolved Hide resolved
integration_tests/compute_runtime/main_test.go Outdated Show resolved Hide resolved
internal/abi/fastly/compute_runtime_guest.go Show resolved Hide resolved
@joeshaw
Copy link
Member

joeshaw commented Aug 19, 2024

The test failures here make me wonder if we need to bump our Viceroy requirement?

Co-authored-by: Federico G. Schwindt <fgsch@users.noreply.github.com>
@dgryski
Copy link
Member

dgryski commented Aug 30, 2024

The timout during the test makes me think it might be a scheduler issue where the process isn't running asynchronously because something is stalled in the sleep. I can look into this more.

@acw acw requested a review from cee-dub August 30, 2024 20:26
@cee-dub cee-dub merged commit 9583372 into main Sep 3, 2024
11 checks passed
@cee-dub cee-dub deleted the fastly/awick/get-vcpu-ms branch September 3, 2024 18:41
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.

5 participants