Project

General

Profile

Actions

Emulator Issues #6097

closed

Code review of jsd-opt-tmp branch

Added by james.jdunne about 11 years ago.

Status:
Fixed
Priority:
High
Assignee:
% Done:

0%

Operating system:
N/A
Issue type:
Other
Milestone:
Regression:
No
Relates to usability:
No
Relates to performance:
No
Easy:
No
Relates to maintainability:
No
Regression start:
Fixed in:

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

Actions #1

Updated by delroth about 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.

Actions #2

Updated by james.jdunne about 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.

Actions #3

Updated by james.jdunne about 11 years ago

  • Status changed from New to Fixed
Actions

Also available in: Atom PDF