Handle VR frame delimiters #1138
Conversation
|
CI gfxreconstruct build queued with queue ID 6966. |
|
CI gfxreconstruct build # 2811 running. |
|
CI gfxreconstruct build # 2811 passed. |
davidd-lunarg
force-pushed
the
davidd-frame-delimiter
branch
from
daa3f4f to
3165d94
Compare
|
CI gfxreconstruct build queued with queue ID 9654. |
|
CI gfxreconstruct build # 2818 running. |
|
CI gfxreconstruct build # 2818 passed. |
davidd-lunarg
force-pushed
the
davidd-frame-delimiter
branch
from
3165d94 to
caf93a4
Compare
|
CI gfxreconstruct build queued with queue ID 9735. |
|
CI gfxreconstruct build # 2819 running. |
|
CI gfxreconstruct build # 2819 passed. |
davidd-lunarg
force-pushed
the
davidd-frame-delimiter
branch
from
caf93a4 to
c81c79a
Compare
|
CI gfxreconstruct build queued with queue ID 17215. |
|
CI gfxreconstruct build # 2859 running. |
davidd-lunarg
force-pushed
the
davidd-frame-delimiter
branch
from
c81c79a to
0d39c31
Compare
|
CI gfxreconstruct build queued with queue ID 17249. |
|
CI gfxreconstruct build # 2861 running. |
|
CI gfxreconstruct build # 2861 passed. |
layer/trace_layer.cpp
Outdated
| std::unordered_set<std::string> kProvidedDeviceFunctions = { "vkCmdDebugMarkerBeginEXT", | ||
| "vkCmdDebugMarkerEndEXT", | ||
| "vkCmdDebugMarkerInsertEXT", | ||
| "vkDebugMarkerSetObjectNameEXT", | ||
| "vkDebugMarkerSetObjectTagEXT" }; | ||
|
|
There was a problem hiding this comment.
Why not const? Also, is 5 entries really enough entries for the asymptotic advantage of an unordered set to beat the memory coherence / simple code advantage of a linear scan of an array? I made a little benchmark to compare linear scan versus set.
Results
- GCC 11.2, O2: https://quick-bench.com/q/KJX1nPqk2ChuLdimDLjMLqwgJds
- GCC 11.2, O3: https://quick-bench.com/q/V7DQR2Mf5qQf9ZZvtBZwZoTSF9A
Listing
#include <unordered_set>
const std::unordered_set<std::string> kProvidedDeviceFunctions = { "vkCmdDebugMarkerBeginEXT",
"vkCmdDebugMarkerEndEXT",
"vkCmdDebugMarkerInsertEXT",
"vkDebugMarkerSetObjectNameEXT",
"vkDebugMarkerSetObjectTagEXT" };
const std::string kProvidedDeviceFunctionsArray[] = { "vkCmdDebugMarkerBeginEXT",
"vkCmdDebugMarkerEndEXT",
"vkCmdDebugMarkerInsertEXT",
"vkDebugMarkerSetObjectNameEXT",
"vkDebugMarkerSetObjectTagEXT" };
constexpr int ITERS { 10000 };
std::vector<std::string> inputs;
const auto& init = [] () {
for(int i = 0; i < ITERS; ++i)
{
for(const std::string& in : kProvidedDeviceFunctionsArray){
inputs.push_back(in);
}
}
std::random_shuffle(inputs.begin(), inputs.end());
return true;
}();
static void TouchInputs(benchmark::State& state) {
// Code inside this loop is measured repeatedly
for (auto _ : state) {
for(const auto& in : inputs){
benchmark::DoNotOptimize(in);
}
}
}
BENCHMARK(TouchInputs);
static void LinearScanArray(benchmark::State& state) {
// Code inside this loop is measured repeatedly
for (auto _ : state) {
for(const auto& in : inputs){
bool found = std::find(std::begin(kProvidedDeviceFunctionsArray), std::end(kProvidedDeviceFunctionsArray), in) != std::end(kProvidedDeviceFunctionsArray);
benchmark::DoNotOptimize(found);
}
}
}
BENCHMARK(LinearScanArray);
static void AmortisedConstantTimeSet(benchmark::State& state) {
// Code inside this loop is measured repeatedly
for (auto _ : state) {
for(const auto& in : inputs){
bool found = (kProvidedDeviceFunctions.find(in) != kProvidedDeviceFunctions.end());
benchmark::DoNotOptimize(found);
}
}
}
BENCHMARK(AmortisedConstantTimeSet);
static void HotInputLinearScanArray(benchmark::State& state) {
// Code inside this loop is measured repeatedly
for (auto _ : state) {
for(int i = 0; i < ITERS; ++i)
{
for(const std::string& in : kProvidedDeviceFunctionsArray){
bool found = std::find(std::begin(kProvidedDeviceFunctionsArray), std::end(kProvidedDeviceFunctionsArray), in) != std::end(kProvidedDeviceFunctionsArray);
benchmark::DoNotOptimize(found);
}
}
}
}
BENCHMARK(HotInputLinearScanArray);
static void HotInputAmortisedConstantTimeSet(benchmark::State& state) {
// Code inside this loop is measured repeatedly
for (auto _ : state) {
for(int i = 0; i < ITERS; ++i)
{
for(const std::string& in : kProvidedDeviceFunctionsArray){
bool found = (kProvidedDeviceFunctions.find(in) != kProvidedDeviceFunctions.end());
benchmark::DoNotOptimize(found);
}
}
}
}
BENCHMARK(HotInputAmortisedConstantTimeSet);
There was a problem hiding this comment.
find(...) != ...end() on a set is idiomatic and seems fine to me. This array could get longer, so we'd have to worry about where the crossover point is, and you'd have to verify clang and MSVC also behave the same way. While I commend your thoroughness including writing a test case in QuickBench (I learned that exists today!), we have a lot of work to do and I don't think this change important enough to microbenchmark something that occurs once at application startup.
I agree it could be const.
My only thought about this new variable would be that in the functions below, kProvidedDeviceFunctions.count(in) > 0 is slightly more readable to me until unordered_set::contains in C++20.
| @@ -327,6 +327,6 @@ def make_cmd_decl(self, return_type, proto, values, name): | |||
| .format(return_type) | |||
| ) | |||
| return_value = '{}{{}}'.format(return_type) | |||
| return 'static {}({}) {{ GFXRECON_LOG_WARNING("Unsupported function {} was called, resulting in no-op behavior."); return {}; }}'.format( | |||
| return 'static {}({}) {{ GFXRECON_LOG_WARNING_ONCE("Unsupported function {} was called, resulting in no-op behavior."); return {}; }}'.format( | |||
There was a problem hiding this comment.
These look great but they make a massive change in framework/generated/generated_vulkan_dispatch_table.h that seems unrelated to the rest of the commit this lives in. Not asking for the containing commit to be split just wondering if (and only if) it were easy to change and commits were being reorged before merging, whether this could be made live in its own commit for logical separation and cleaner atomic revert if ever needed.
| @@ -3329,6 +3366,44 @@ VkResult VulkanReplayConsumerBase::OverrideQueueSubmit2(PFN_vkQueueSubmit2 func, | |||
| GetDeviceTable(queue_info->handle)->QueueWaitIdle(queue_info->handle); | |||
| } | |||
|
|
|||
| // Check whether any of the submitted command buffers are frame boundaries. | |||
| if (screenshot_handler_ != nullptr) | |||
| { | |||
There was a problem hiding this comment.
This isn't the other place I saw a search of command buffers. I saw it in another file too.
|
Is there anything in CI that tests this? I think CI is currently just going to test that it doesn't break anything when the markers are not in use? |
|
CI gfxreconstruct build queued with queue ID 1766. |
|
CI gfxreconstruct build # 2895 running. |
|
CI gfxreconstruct build # 2895 passed. |
That's right, CI doesn't currently have any tests that contain the VR frame markers. |
davidd-lunarg
force-pushed
the
davidd-frame-delimiter
branch
from
a790fa1 to
57abf89
Compare
|
CI gfxreconstruct build queued with queue ID 3352. |
|
CI gfxreconstruct build # 2910 running. |
|
CI gfxreconstruct build # 2910 passed. |
davidd-lunarg
force-pushed
the
davidd-frame-delimiter
branch
from
57abf89 to
0970ba9
Compare
|
CI gfxreconstruct build queued with queue ID 4632. |
|
CI gfxreconstruct build # 2962 running. |
|
CI gfxreconstruct build # 2962 passed. |
Also commit change to `generated_vulkan_export_json_consumer.cpp` that was made when running generate_vulkan.py
Some VR applications use debug markers to identify frame boundaries but not all VR devices provide support for VK_EXT_debug_marker. With this change GFXR provides function pointers for the extension's functions in order to capture the debug marker calls and track VR frames.
davidd-lunarg
force-pushed
the
davidd-frame-delimiter
branch
from
0970ba9 to
fb4b9a0
Compare
|
CI gfxreconstruct build queued with queue ID 9251. |
|
CI gfxreconstruct build # 2997 running. |
|
CI gfxreconstruct build # 2997 passed. |
When a command buffer containing a VR end of frame debug marker is submitted, treat that as a frame delimiter. During capture, write end of frame markers to the capture file when this special end of frame condition is encountered and handle those frame markers in the file processor.