BestPractices-SyncObjects-HighNumberOfFences : off by one error

[filnet]

Environment:

• OS: Windows
• GPU and driver version: NVidia
• SDK or header version if building from repo: 1.3.290.0
• Options enabled (synchronization, best practices, etc.): best practices

Describe the Issue

The BestPractices-SyncObjects-HighNumberOfFences warning triggers after creating the fifth fence and not after creating the fourth (the limit is defined as 3). The check is done in a PreCallValidateCreateFence so maybe before the count is updated...

Side note: would be nice if the warning message included the limit and the current count.

PS: other such checks might have the same issue.

[filnet]

This check seems a bit hard to not trigger as fences are not used only for in flight frames... I allow only two in flight frames so have only 2 fences for that. But I also use fences for waiting for queued commands to finish executing. So currently I have 7 fences and trigger this warning.

[spencer-lunarg]

Made a PR to fix the error message, not fully sure if there is threading issues or what, but the VkAmdBestPracticesLayerTest.NumSyncPrimitives test shows we report on the 4th fence there

[filnet]

My code is not multi threaded during the initialization phase. I added a debug printf prior to every call to create_fence and the output shows that the warning is emitted on the 5th call and thereafter. The warning is emmitted 3 times for 7 created fences. So one short.

	Line 111: CREATE FENCE
	Line 113: CREATE FENCE
	Line 193: CREATE FENCE
	Line 234: CREATE FENCE
	Line 377: CREATE FENCE
	Line 378: [Warning][Performance]"Validation Performance Warning: [ BestPractices-SyncObjects-HighNumberOfFences ] | MessageID = 0x
	Line 379: a9f4ff68 | vkCreateFence():  [AMD] [NVIDIA] High number of VkFence objects created.Minimize the amount of CPU-GPU synchr
	Line 380: onization that is used. Semaphores and fences have overhead. Each fence has a CPU and GPU cost with it."
	Line 381: CREATE FENCE
	Line 382: [Warning][Performance]"Validation Performance Warning: [ BestPractices-SyncObjects-HighNumberOfFences ] | MessageID = 0x
	Line 383: a9f4ff68 | vkCreateFence():  [AMD] [NVIDIA] High number of VkFence objects created.Minimize the amount of CPU-GPU synchr
	Line 384: onization that is used. Semaphores and fences have overhead. Each fence has a CPU and GPU cost with it."
	Line 388: CREATE FENCE
	Line 389: [Warning][Performance]"Validation Performance Warning: [ BestPractices-SyncObjects-HighNumberOfFences ] | MessageID = 0x
	Line 390: a9f4ff68 | vkCreateFence():  [AMD] [NVIDIA] High number of VkFence objects created.Minimize the amount of CPU-GPU synchr
	Line 391: onization that is used. Semaphores and fences have overhead. Each fence has a CPU and GPU cost with it."

EDIT: I also added debug printf when destroying fences. All seven fences are destroyed when exiting (i.e. there are no short lived fences in the initialization phase).

[filnet]

My hunch is that the hook that checks the number of created fences is called before the hook that increments the count.

[filnet]

And the test expects the warning on the 5th fence creation (and not the 4th as desired)..

The loop creates 4 fences and then a fifth is created to trigger the warning:

    constexpr int fence_warn_limit = 5;
    const auto fence_ci = vkt::Fence::create_info();
    std::vector<vkt::Fence> test_fences(fence_warn_limit);
    for (int i = 0; i < fence_warn_limit - 1; ++i) {
        test_fences[i].init(*m_device, fence_ci);
    }
    m_errorMonitor->SetDesiredFailureMsg(kPerformanceWarningBit, "BestPractices-SyncObjects-HighNumberOfFences");
    test_fences[fence_warn_limit - 1].init(*m_device, fence_ci);
    m_errorMonitor->VerifyFound();

fence_warn_limit should be set to kMaxRecommendedFenceObjectsSizeAMD + 1 (i.e. 4)