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

layers: Add BASE_NODE tree locking #3658

Merged

Conversation

jeremyg-lunarg
Copy link
Contributor

Maintaining the tree of which objects are in use by command buffers is performance critical and also likely to cause interactions between threads. Make access to the tree thread safe by guarding each nodes parent_nodes_ set with a separate r/w lock.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 11910.

@jeremyg-lunarg jeremyg-lunarg marked this pull request as draft January 7, 2022 14:53
@jeremyg-lunarg
Copy link
Contributor Author

WIP: this depends on #3657 and needs a much better description of what is happening

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 5858 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 5858 passed.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 15039.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 5909 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 5909 passed.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 15435.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 5923 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 5923 passed.

@jeremyg-lunarg jeremyg-lunarg marked this pull request as ready for review January 13, 2022 01:51
Copy link
Contributor

@ncesario-lunarg ncesario-lunarg left a comment

Choose a reason for hiding this comment

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

One naming question, but LGTM!

layers/base_node.cpp Outdated Show resolved Hide resolved
layers/base_node.cpp Show resolved Hide resolved
layers/base_node.h Show resolved Hide resolved
Copy link
Contributor

@jzulauf-lunarg jzulauf-lunarg left a comment

Choose a reason for hiding this comment

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

A couple questions...

layers/base_node.cpp Show resolved Hide resolved
layers/base_node.cpp Show resolved Hide resolved
layers/base_node.cpp Show resolved Hide resolved
layers/base_node.cpp Outdated Show resolved Hide resolved
layers/base_node.cpp Show resolved Hide resolved
layers/base_node.h Show resolved Hide resolved
layers/best_practices_utils.cpp Show resolved Hide resolved
layers/descriptor_sets.cpp Show resolved Hide resolved
layers/image_state.h Outdated Show resolved Hide resolved
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 16608.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 5942 running.

Maintaining the tree of which objects are in use by command buffers is
performance critical and also likely to cause interactions between
threads. Make access to the tree thread safe by guarding each
nodes parent_nodes_ set with a separate r/w lock.
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 16622.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 5945 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 5945 passed.

Copy link
Contributor

@jzulauf-lunarg jzulauf-lunarg left a comment

Choose a reason for hiding this comment

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

LGTM

@jeremyg-lunarg jeremyg-lunarg merged commit 610d3a6 into KhronosGroup:master Jan 14, 2022
@jeremyg-lunarg jeremyg-lunarg deleted the jeremyg-tree-locking branch January 14, 2022 17:29
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