Emulator Issues #6097
closedCode review of jsd-opt-tmp branch
0%
Description
Purpose of code changes on this branch:
Optimizations according to profiler findings
Reducing number of PeekMessage() calls from RunGpuLoop
Eliminating YieldCPU() calls from RunGpuLoop
Eliminating expensive Core::IsGPUThread() calls from SetCpStatus
When reviewing my code changes, please focus on:
Focus on commits [bb504f3..91ee3c7]
Ignore VS2012 updates to project files; those will not be merged.
After the review, I'll merge this branch into:
master
Updated by delroth over 11 years ago
Changes 2 and 3 LGTM. Thanks for the benchmarks.
I'm a bit more worried about the effects of change 1. This might delay message handling a lot if the GPU thread is busy and continuously has messages to handle (when Dolphin is GPU bound, for example). Could you check if it has a real performance impact? If it does not I think it would be better to not include it.
Updated by james.jdunne over 11 years ago
Understandable. According to my profiler, PeekMessageW() was getting called an awful lot of times, way more than necessary IMHO for normal UI message pumping. If the thread is hung up in the GPU driver, more calls to PeekMessageW() aren't necessarily going to help.
I agree that it is a concern, and the 0x1FFFF number is rather arbitrary, so I would feel better too with some more rigor behind that number. But I don't think it should just be left alone to continually pump the message loop for every iteration of RunGpuLoop.
What sorts of UI messages (besides obvious WM_QUIT and regular GUI events) are expected to be handled in a timely manner? Are there any WM_TIMER or mouse/keyboard/joystick/wiimote input messages required for emulation here? I would assume those sorts of things are handled by other means. During my play-testing I didn't notice any change in controller lag, but I certainly didn't exercise the input system with UI features.
It would be better (if it were possible) to ensure that the GPU thread is separate from the UI thread so that the message pump can run at a much lower frequency than the GPU updates, but I think some graphics APIs make this impossible unfortunately (looking at you, OpenGL).
For the time being I'll cut a new branch with the PeekMessage throttling code removed and merge that up into master.
Thanks for taking the time to review! Much appreciated.