Vsync proposal #4133
Open
Vsync proposal #4133
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
This abstracts pthread_mutex_t and pthread_cond_t entirely and lives in libvarnish making it usable outside of varnishd. Instrumentation is abstracted and is externally supplied. This also exposes the ability to use the monotonic clock to wait on conditions, macOS <= 14.4 appear to not have the POSIX API allowing this, a transparent portability trade-off is proposed for this case.
asadsa92
reviewed
Jul 8, 2024
Comment on lines
+42
to
+45
| .. varnish_vsc:: wait_duration_ns | ||
| :type: counter | ||
| :level: debug | ||
| :oneliner: Amount of nanoseconds spent waiting of condition variables. |
There was a problem hiding this comment.
fyi, an attempt at this was rejected last time around: #3977
Not sure if adding this to non-core locks is OK.
There was a problem hiding this comment.
This amount of time is an amount of time spent waiting on a condition, it is not related to locks per se. I would therefore argue that it is not the same. Isn't the change you mention measures the amount of time in a critical section ?
There was a problem hiding this comment.
It was both, time spent waiting for the lock or condvar.
As I said, it could be OK outside of the core. I just mentioned it as it was closely related.
|
The test failure seems to come from a flaky test (I am not reproducing the issue nor do I see how it could relate to this PR). |
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.
This proposes an API to abstract pthread synchronization devices (mutex & conditions) as part of libvarnish making it usable outside of varnishd while offering the ability to leverage varnishd VSC instrumentation.