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

syncval: Split Device and Instance ValidationObjects #9235

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jeremyg-lunarg
Copy link
Contributor

This is the start of splitting class ValidationObject into separate Device and Instance classes. Syncval has nothing happening at Instance level so it gets to go first. Once all the leaf classes are split, ValidationStateTracker and ValidationObject can be split. This seems backwards but allows it to happen incrementally rather than in 1 giant PR

@jeremyg-lunarg jeremyg-lunarg requested a review from a team as a code owner January 16, 2025 17:28
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 347528.

This is the start of splitting class ValidationObject into
separate Device and Instance classes. Syncval has nothing happening
at Instance level so it gets to go first. Once all the leaf
classes are split, ValidationStateTracker and ValidationObject
can be split. This seems backwards but allows it to happen
incrementally rather than in 1 giant PR
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 347543.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18711 running.

@@ -27,6 +27,14 @@
#include "sync/sync_stats.h"
#include "sync/sync_submit.h"

namespace syncval {
// sync validation has no instance-level functionality
class Instance : public ValidationStateTracker {
Copy link
Contributor

@artem-lunarg artem-lunarg Jan 16, 2025

Choose a reason for hiding this comment

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

Got confused that comment says no instance functionality but also create Instance class. In what direction this is going to evolve? Do we need at least empty Instance class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There will be a couple of phases:

  1. First this will inherit from the Instance object of the state tracker, something like vvl::state::Instance
  2. Then there will be a common state tracker and I think this class will go away. Since syncval does nothing here, your 'device' level objects (SyncValidators) will just point at the shared state trackers Device and Instance objects

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other validation types will still have to have Instance classes so that they can do their current instance level functionality.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18711 failed.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 348126.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18728 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18728 passed.

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.

3 participants