Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gpuav: Improve shader map management #9307

Conversation

arno-lunarg
Copy link
Contributor

Deleting an element from instrumented_shaders_map_ was too slow

@arno-lunarg arno-lunarg requested a review from a team as a code owner January 24, 2025 17:10
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 355210.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18851 running.

@@ -52,6 +52,7 @@ struct ShaderObject : public StateObject {
// TOOD Create a shader object inherited class
struct InstrumentationData {
bool was_instrumented = false;
uint32_t unique_shader_id = 0xffff'ffff;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unique_shader_id of 0 is used in IsInstrumented to mean "not valid"

so just use 0, it is already defined as the "invalid" value

@@ -138,7 +138,7 @@ class Pipeline : public StateObject {
// ment for GPU-AV
struct InstrumentationData {
// We create a VkShaderModule that is instrumented and need to delete before leaving the pipeline call
std::vector<VkShaderModule> instrumented_shader_module;
std::vector<std::pair<uint32_t, VkShaderModule>> instrumented_shader_modules;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment what this pair is

if (auto pipeline_state = Get<vvl::Pipeline>(pipeline)) {
for (auto shader_module : pipeline_state->instrumentation_data.instrumented_shader_module) {
DispatchDestroyShaderModule(device, shader_module, pAllocator);
for (auto shader_module : pipeline_state->instrumentation_data.instrumented_shader_modules) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we do the auto [unique_shader_id, shader_module_handle] syntax because it is a std::pair?

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18851 passed.

Deleting an element from instrumented_shaders_map_ was too slow
@arno-lunarg arno-lunarg force-pushed the arno-gpuav-instrumentated-shaders-map-updates-rework branch from dead474 to 3c0e7d6 Compare January 24, 2025 17:56
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 355292.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18853 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18853 passed.

@arno-lunarg arno-lunarg merged commit e70f19c into KhronosGroup:main Jan 24, 2025
20 checks passed
@arno-lunarg arno-lunarg deleted the arno-gpuav-instrumentated-shaders-map-updates-rework branch January 24, 2025 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants