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

OpenVR latency via windows pipes is worse than OSC #511

Closed
kitlith opened this issue Jan 24, 2023 · 13 comments
Closed

OpenVR latency via windows pipes is worse than OSC #511

kitlith opened this issue Jan 24, 2023 · 13 comments
Labels
Area: SteamVR Driver Related to the SteamVR Driver Priority: High Important feature or blocks something important Type: Discussion Further information is requested
Milestone

Comments

@kitlith
Copy link
Member

kitlith commented Jan 24, 2023

This is a companion issue to SlimeVR/SlimeVR-OpenVR-Driver#34, which tracks how we can improve latency in the openvr driver.

  • OpenVR: messages are sent to a queue which is polled at 200-ish hz from another thread
    • (I linked to the windows named pipes version, but the unix socket version works the same way)
    • This probably accounts for some of the latency I measured using https://github.com/kitlith/slimevr-latency (which ranged from 6ms to 12ms on my machine, though I should set something up to aggregate and do statistics to it.)
  • OSC: messages are sent immediately, barring any complications inside the OSC library that I've not investigated.

This probably causes at least a few ms (and possibly mildly inconsistent) latency for openvr that is not present for OSC. We might be able to improve this by removing the thread, and try to use non-blocking IO to do all the network/IPC in parallel. Otherwise, we could schedule the thread to wake when we push data to the queue instead of having it poll at a fixed 200-ish hz. (the latter is probably a quicker immediate fix, the former may require changing server architecture. shrug)

@kitlith
Copy link
Member Author

kitlith commented Jan 25, 2023

I have confirmed that the thread polling at 200hz is responsible for some extra latency by replacing the Thread.sleep(5) with a Thread.yield() locally. This reduces latency down to 0.9-3 ms in most cases, with the occasional outlier of >10ms (probably due to thread scheduling?)

EDIT: this seems to be about the same (maybe slightly quicker on average) as if I remove the Thread.yield() and just busy-loop. wonder if thread scheduling of my measurement tool isn't helping. shrug

I tried setting it up so that instead of using Thread.sleep(5), we block on the outgoing message queue for up to 5ms. Unfortunately, that seems to either do nothing, or increase the latency we experience.

@kitlith
Copy link
Member Author

kitlith commented Jan 25, 2023

I've updated my latency measuring tool to also listen to OSC. Can confirm, currently OSC consistently has 3-4ms less latency than sending to the driver.

Replacing Thread.sleep(5) with Thread.yield() brings OSC and OpenVR to have almost identical latency of 1-4ms (it's random which one recieves first)

EDIT: it's probably worth also taking a look at what's pulling things off of the recieve queue for the bridge, I wonder if that's another source of latency for HMD positions coming from the driver (and would apply to both OSC and OpenVR in my existing benchmarks, because I don't have a test setup for the tracker protocol yet)

EDIT 2: replacing the sleep in the VRServer main loop with a yield (or doing sleep(0) or similar) seems to make the driver thread turn really slow. but osc remains fast! ish. I wonder if some of the results I'm seeing are a result of issues with my measurement tool. :/

@Eirenliel
Copy link
Member

the thread is at 200Hz because the driver should update at hmd update rate which is usually 144Hz or lower. we can set the sleep to 1ms it should give more consistent result than THread.yield() with less CPU load.

@kitlith
Copy link
Member Author

kitlith commented Jan 25, 2023

we can set the sleep to 1ms it should give more consistent result than Thread.yield() with less CPU load.

Well, it consistently has about 3ms more latency than OSC if you put any time in Thead.sleep() other than 0, which is equivalent to Thread.yield()

I worked for a few hours on a version of the bridge that uses overlapped IO instead of polling, with the idea being "maybe get the best of both worlds." I can push that somewhere if other people are also interested in working on it. I need another break from it though. I feel like there's got to be a library somewhere that wraps all this up into a nice async interface that i'm missing.

EDIT: https://github.com/sbt/ipcsocket might be an option i guess. except kinda not really since that implements a socket, not a channel.

give me my sweet, sweet, rust

@kitlith
Copy link
Member Author

kitlith commented Jan 26, 2023

Okay, question. Would it make sense the way that the server is currently built to turn this thread into one that pulls information from the skeleton instead of having data pushed to it? That way, this thread could be simplified into: (pseudocode)

while True:
    hmd_info = read_hmd_packet() # blocking
    copy_skeleton() # or make a copy or something, so it isn't being concurrently modified
    update_skeleton(hmd_info) # only does the minimum computations necessary to account for current hmd position
    send_trackers_to_driver() # blocking

driver would then go: (pseudocode)

def driver_frame():
    send_hmd_info() # blocking
    recv_trackers() # blocking

we could extend this idea to also include the trackers from feeder app with some extra work. (feeder would no longer feed positions, just role information directly to driver. something something I finally cave to making feeder a thread or subprocess of driver)

This would kinda kill two birds with one stone. This issue, since we're always just waiting on the OS, and the issue on the driver side, because we're always sending up to date information.

@ButterscotchV ButterscotchV added Type: Discussion Further information is requested Area: SteamVR Driver Related to the SteamVR Driver labels Jan 30, 2023
@TheButlah TheButlah mentioned this issue Feb 12, 2023
17 tasks
@TheButlah TheButlah added this to the SlimeVR v1.0 milestone Feb 12, 2023
@TheButlah TheButlah added the Priority: High Important feature or blocks something important label Feb 12, 2023
@ImUrX
Copy link
Member

ImUrX commented Feb 24, 2023

#607 added polling to the Windows pipes, remaining is Linux 😭

@kitlith
Copy link
Member Author

kitlith commented Feb 24, 2023

More specifically, that PR moves from sleeping to waiting for an incoming message with timeout. It prioritizes listening to the driver over sending updates from the server. (I'd like to build something that wake up immediately in both cases, but this is probably a step in the right direction.)

@TheButlah
Copy link
Contributor

TheButlah commented Mar 13, 2023

I think this can be closed now. The latency is good enough to where I now prefer steamvr over osc. I'm sure more optimizations will be done in the future eventually.

If there are any specific things that we know we can still optimize, maybe we could open issues for those?

@ImUrX ImUrX reopened this Mar 13, 2023
@ImUrX
Copy link
Member

ImUrX commented Mar 13, 2023

This was only implemented on windows

@TheButlah
Copy link
Contributor

TheButlah commented Mar 13, 2023

Have you tested it yourself to know if the latency is even a problem on linux? I would open a new issue for linux, as this issue is discussion about windows named pipes and blocks #577. Linux does not need to block 1.0.

@TheButlah TheButlah changed the title OpenVR latency is worse than OSC OpenVR latency is worse than OSC on windows Mar 13, 2023
@TheButlah TheButlah changed the title OpenVR latency is worse than OSC on windows OpenVR latency via windowsnamedpipes is worse than OSC Mar 13, 2023
@TheButlah TheButlah changed the title OpenVR latency via windowsnamedpipes is worse than OSC OpenVR latency via windows pipes is worse than OSC Mar 13, 2023
@ImUrX
Copy link
Member

ImUrX commented Mar 13, 2023

Have you tested it yourself to know if the latency is even a problem on linux? I would open a new issue for linux, as this issue is discussion about windows named pipes and blocks #577. Linux does not need to block 1.0.

aside from you specifying this is windows, I dont see any place that speaks about windows-specific code :/

@TheButlah
Copy link
Contributor

TheButlah commented Mar 13, 2023

Literally all of the linked performance research is about windows named pipes, the discussion that prompted this issue was due to performance observed on windows, and the original motivation for linking the 1.0 milestone is to ensure the experience for reviewers is good, which means windows.

I appreciate the effort to improve linux, but that simply is not the focus of this discussion. If you want to open an issue for that, then you need to actually run it on linux and demonstrate that there is a problem with latency that can be improved.

@kitlith
Copy link
Member Author

kitlith commented Mar 14, 2023

If you want to open an issue for that, then you need to actually run it on linux and demonstrate that there is a problem with latency that can be improved.

This was, in fact, an issue on linux, which will be fixed in #629, which also fixes an bug in the previous PR that left the server submitting data a frame behind on windows. However, I had brought up that bug directly on the older PR, not in this issue.

I'm not going to bother re-opening since that PR should fix it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: SteamVR Driver Related to the SteamVR Driver Priority: High Important feature or blocks something important Type: Discussion Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants