Skip to content

Commit ba774d9

Browse files
committed
layers: Cleanup submit time tracker
Make validity check part of timeline queries. In the initial version CanBeResolved missed semaphore validity check
1 parent 2c8dd21 commit ba774d9

File tree

2 files changed

+23
-18
lines changed

2 files changed

+23
-18
lines changed

layers/state_tracker/submit_time_tracker.cpp

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,6 @@ bool SubmitTimeTracker::ProcessBatch(std::vector<std::shared_ptr<CommandBuffer>>
111111

112112
bool SubmitTimeTracker::ProcessSignal(VkSemaphore timeline, uint64_t signal_value) {
113113
bool skip = false;
114-
auto semaphore = validator_.Get<Semaphore>(timeline);
115-
if (!semaphore || semaphore->type != VK_SEMAPHORE_TYPE_TIMELINE) {
116-
return skip;
117-
}
118114
const bool new_timeline_signal = UpdateTimelineValue(timeline, signal_value);
119115
if (new_timeline_signal) {
120116
skip |= PropagateTimelineSignals();
@@ -126,14 +122,12 @@ std::vector<VkSemaphoreSubmitInfo> SubmitTimeTracker::GetUnresolvedTimelineWaits
126122
vvl::span<const VkSemaphoreSubmitInfo> wait_semaphores) {
127123
std::vector<VkSemaphoreSubmitInfo> unresolved;
128124
for (const auto& wait : wait_semaphores) {
129-
auto semaphore = validator_.Get<Semaphore>(wait.semaphore);
130-
if (!semaphore || semaphore->type != VK_SEMAPHORE_TYPE_TIMELINE || semaphore->Scope() != Semaphore::kInternal) {
131-
// Only timeline semaphores can introduce pending waits.
132-
// For external semaphores we cannot reliably track signals
125+
const std::optional<uint64_t> current_value = GetTimelineValue(wait.semaphore);
126+
if (!current_value.has_value()) {
127+
// Invalid or external semaphores should not block this batch
133128
continue;
134129
}
135-
const uint64_t current_value = GetTimelineValue(wait.semaphore);
136-
if (wait.value > current_value) {
130+
if (wait.value > *current_value) {
137131
unresolved.emplace_back(wait);
138132
}
139133
}
@@ -143,10 +137,6 @@ std::vector<VkSemaphoreSubmitInfo> SubmitTimeTracker::GetUnresolvedTimelineWaits
143137
bool SubmitTimeTracker::RegisterTimelineSignals(vvl::span<const VkSemaphoreSubmitInfo> signal_semaphores) {
144138
bool new_timeline_signals = false;
145139
for (const VkSemaphoreSubmitInfo& signal : signal_semaphores) {
146-
auto semaphore = validator_.Get<Semaphore>(signal.semaphore);
147-
if (!semaphore || semaphore->type != VK_SEMAPHORE_TYPE_TIMELINE) {
148-
continue;
149-
}
150140
new_timeline_signals |= UpdateTimelineValue(signal.semaphore, signal.value);
151141
}
152142
return new_timeline_signals;
@@ -180,20 +170,34 @@ bool SubmitTimeTracker::PropagateTimelineSignals() {
180170

181171
bool SubmitTimeTracker::CanBeResolved(const UnresolvedBatch& batch) const {
182172
for (const VkSemaphoreSubmitInfo& wait : batch.unresolved_timeline_waits) {
183-
const uint64_t current_value = GetTimelineValue(wait.semaphore);
173+
const std::optional<uint64_t> current_value = GetTimelineValue(wait.semaphore);
174+
if (!current_value.has_value()) {
175+
// Invalid or external semaphores should not block this batch
176+
continue;
177+
}
184178
if (wait.value > current_value) {
185179
return false;
186180
}
187181
}
188182
return true;
189183
}
190184

191-
uint64_t SubmitTimeTracker::GetTimelineValue(VkSemaphore timeline) const {
185+
std::optional<uint64_t> SubmitTimeTracker::GetTimelineValue(VkSemaphore timeline) const {
186+
auto semaphore_state = validator_.Get<Semaphore>(timeline);
187+
if (!semaphore_state || semaphore_state->type != VK_SEMAPHORE_TYPE_TIMELINE ||
188+
semaphore_state->Scope() != Semaphore::kInternal) {
189+
// Used by the caller to detect invalid/non-timeline/external semaphores
190+
return {};
191+
}
192192
const uint64_t current_value = vvl::FindExisting(timeline_signals_, timeline);
193193
return current_value;
194194
}
195195

196196
bool SubmitTimeTracker::UpdateTimelineValue(VkSemaphore timeline, uint64_t signal_value) {
197+
auto semaphore_state = validator_.Get<Semaphore>(timeline);
198+
if (!semaphore_state || semaphore_state->type != VK_SEMAPHORE_TYPE_TIMELINE) {
199+
return false;
200+
}
197201
uint64_t& current_value = vvl::FindExisting(timeline_signals_, timeline);
198202
if (signal_value <= current_value) {
199203
return false; // non-increasing signal, the error should be reported elsewhere

layers/state_tracker/submit_time_tracker.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <vulkan/vulkan.h>
2323
#include <memory>
2424
#include <mutex>
25+
#include <optional>
2526
#include <vector>
2627

2728
namespace vvl {
@@ -64,7 +65,7 @@ class SubmitTimeTracker {
6465
bool ProcessPresent(const VkPresentInfoKHR& present_info, const Location& present_info_loc) const;
6566

6667
private:
67-
bool ProcessBatch(std::vector<std::shared_ptr<vvl::CommandBuffer>>&& command_buffers,
68+
bool ProcessBatch(std::vector<std::shared_ptr<CommandBuffer>>&& command_buffers,
6869
vvl::span<const VkSemaphoreSubmitInfo> wait_semaphores,
6970
vvl::span<const VkSemaphoreSubmitInfo> signal_semaphores, VkQueue queue, const Location& submit_loc);
7071
bool ProcessSignal(VkSemaphore timeline, uint64_t signal_value);
@@ -73,7 +74,7 @@ class SubmitTimeTracker {
7374
bool RegisterTimelineSignals(vvl::span<const VkSemaphoreSubmitInfo> signal_semaphores);
7475
bool PropagateTimelineSignals();
7576
bool CanBeResolved(const UnresolvedBatch& batch) const;
76-
uint64_t GetTimelineValue(VkSemaphore timeline) const;
77+
std::optional<uint64_t> GetTimelineValue(VkSemaphore timeline) const;
7778
bool UpdateTimelineValue(VkSemaphore timeline, uint64_t signal_value);
7879

7980
private:

0 commit comments

Comments
 (0)