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

fix: make android samples monotonic #337

Merged
merged 14 commits into from
Oct 18, 2023

Conversation

viglia
Copy link
Contributor

@viglia viglia commented Oct 17, 2023

Wall-clock time is supposed to be monotonic. In a few rare cases we've noticed this was not the case.

Due to some overflow happening client-side in the embedded profiler, the sequence might be decreasing at certain points.

This is just a workaround to mitigate this issue (at rendering time) and avoid a front-end crash, should it happen.

@viglia viglia self-assigned this Oct 17, 2023
@viglia viglia requested a review from a team as a code owner October 17, 2023 12:30
Copy link
Contributor

@phacops phacops left a comment

Choose a reason for hiding this comment

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

What happens if the Nanos part of the clock is having this behavior? Shouldn't we take care of the whole clock?

internal/profile/android.go Outdated Show resolved Hide resolved
internal/profile/android.go Outdated Show resolved Hide resolved
internal/profile/android.go Outdated Show resolved Hide resolved
internal/profile/android.go Outdated Show resolved Hide resolved
internal/profile/android.go Outdated Show resolved Hide resolved
@viglia
Copy link
Contributor Author

viglia commented Oct 17, 2023

What happens if the Nanos part of the clock is having this behavior? Shouldn't we take care of the whole clock?

@phacops I don't know if this can happen.

@viglia viglia requested a review from phacops October 17, 2023 14:17
@viglia
Copy link
Contributor Author

viglia commented Oct 17, 2023

What happens if the Nanos part of the clock is having this behavior? Shouldn't we take care of the whole clock?

@phacops I don't know if this can happen.

I've changed the logic to convert secs and nanos to a unique duration expressed in ns and act on that instead.

This will take care of everything and it's more appropriate than treating them independently.

Loop once, if the condition is detected then break and loop again
starting from that index applying the logic to fix the time without
checking the condition each time
@viglia viglia requested a review from phacops October 17, 2023 18:22
@viglia viglia merged commit 6a50fe9 into main Oct 18, 2023
11 checks passed
@viglia viglia deleted the viglia/fix/make-android-samples-monotonic branch October 18, 2023 07:30
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