mirror of
https://github.com/ARMSX2/ARMSX3.git
synced 2026-08-24 16:58:52 -07:00
RSX: charge the empty-ring yield to idle, and stop draining the present queue on flush
Two things, both about the RSX waiting rather than working. flush_command_queue ended by draining the present queue in case a queued frame still held a ref to the command buffer just taken. It cannot: next() hands them out from a 512 entry ring and the queued list is bounded at flip to m_max_async_frames - 1, so the buffer being reused is hundreds of frames retired. The guard was unreachable and the cost was not -- check_present_status pokes the oldest queued frame's swap command buffer, and on Adreno vkGetFenceStatus blocks until signalled rather than returning VK_NOT_READY, so a poll written to be cheap became a full GPU sync. 1.32 times a frame at about 11ms: Fence poll 14.6ms -> 0.033ms, frame 44.5ms -> 36.6ms. Same fault as the two sites removed earlier; this one sat inside flush_command_queue rather than on the present path. Ruled out first: identical frame time at quarter resolution, and forcing the swapchain pre-transform to match the surface left it unchanged. The empty-ring yield is now charged to idle. It sits inside fifo_decode, which is the enclosing scope of the whole run loop, so waiting on an empty ring was reported as decode work -- Idle 0.003ms against FIFO decode 20.4ms, while a native profile of the same thread put 34% of its cycles in sched_yield. The bucket report and the profiler disagreed and the bucket report was wrong, which has now produced two wrong conclusions in one session.
This commit is contained in:
@@ -764,6 +764,19 @@ namespace rsx
|
||||
// threads runnable, five idle. Nothing is waiting for the core this would
|
||||
// give back, and the RSX sits on the frame's dependency chain, so sleeping
|
||||
// only delays the moment it notices the guest has produced work.
|
||||
//
|
||||
// Charged to idle, because that is what it is. This yield sits inside the
|
||||
// fifo_decode scope, and fifo_decode is the enclosing scope of the whole run
|
||||
// loop, so without this the time the RSX spends waiting on an empty ring is
|
||||
// reported as decode work. That reads as a saturated RSX -- Idle 0.003 ms
|
||||
// against FIFO decode 20.4 ms -- while a native profile of the same thread
|
||||
// put 34% of its cycles in sched_yield and its kernel path. The bucket
|
||||
// report and the profiler disagreed, and the bucket report was wrong.
|
||||
//
|
||||
// It has now caused two wrong conclusions in one session: once reading a
|
||||
// starving RSX as CPU-bound decode, and once reading a thread stuck in an
|
||||
// occlusion query wait as the same thing.
|
||||
RSX_PROF_SCOPE(idle);
|
||||
std::this_thread::yield();
|
||||
}
|
||||
|
||||
|
||||
@@ -1606,8 +1606,26 @@ void VKGSRender::flush_command_queue(bool hard_sync, bool do_not_switch)
|
||||
ensure(hard_sync);
|
||||
}
|
||||
|
||||
// Just in case a queued frame holds a ref to this cb, drain the present queue
|
||||
check_present_status();
|
||||
// Deliberately NOT draining the present queue here.
|
||||
//
|
||||
// The drain exists in case a queued frame still holds a ref to the command buffer just
|
||||
// taken. It cannot: next() hands them out from a 512 entry ring, and the queued frame list
|
||||
// is bounded at flip to m_max_async_frames - 1, so the buffer being reused is hundreds of
|
||||
// frames retired. The guard is unreachable and the cost is not.
|
||||
//
|
||||
// check_present_status pokes the oldest queued frame's swap command buffer, and on Adreno
|
||||
// vkGetFenceStatus blocks until signalled instead of returning VK_NOT_READY, so a poll that
|
||||
// is written to be cheap becomes a full GPU sync. Called from here it ran 1.32 times a frame
|
||||
// at about 11ms, which is the 14.6ms of Fence poll -- 31% of the frame in Web of Shadows,
|
||||
// second only to the whole RSX decode loop.
|
||||
//
|
||||
// Same fault as the two sites removed with the earlier [wait][record] to [record][wait]
|
||||
// change; this third one was missed because it sits inside flush_command_queue rather than
|
||||
// on the present path. Ruled out first: the frame time is unchanged at quarter resolution,
|
||||
// so it is not GPU work, and forcing the swapchain pre-transform to match the surface left
|
||||
// it at 14.6ms, so it is not compositor rotation either.
|
||||
//
|
||||
// Frames are still retired: the flip path drains and bounds the queue.
|
||||
|
||||
if (m_occlusion_query_active)
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user