syncval: New generic range traversal

Author: jzulauf-lunarg

layers/containers/range_vector.h

@@ -1626,6 +1626,60 @@ Iterator split(Iterator in, Map &map, const Range &range) {
     return pos;
 }
 
+// Apply an operation over a range map, infilling where content is absent, updating where content is present.
+// Trims to range boundaries.
+// infill op doesn't have to alter map, but mustn't invalidate iterators passed to it. (i.e. no erasure)
+// infill data (default mapped value or other initial value) is contained with ops.
+// update allows existing ranges to be updated (merged, whatever) based on data contained in ops.  All iterators
+// passed to update are already trimmed to fit within range.
+template <typename RangeMap, typename InfillUpdateOps>
+void infill_update_range(RangeMap &map, const typename RangeMap::key_type &range, const InfillUpdateOps &ops) {
+    using KeyType = typename RangeMap::key_type;
+    using IndexType = typename RangeMap::index_type;
+    const auto map_end = map.end();
+
+    if (range.empty()) return;
+
+    // Starting with
+    auto pos = map.lower_bound(range);
+    if ((pos != map_end) && (range.begin > pos->first.begin)) {
+        // lower bound starts before the range, trim and advance
+        pos = map.split(pos, range.begin, sparse_container::split_op_keep_both());
+        ++pos;
+    }
+
+    IndexType current_begin = range.begin;
+    while ((pos != map_end) && (current_begin < range.end)) {
+        // The current_begin is either pointing to the next existing value to update or the beginning of a gap to infill
+        assert(pos->first.begin >= current_begin);
+
+        if (current_begin < pos->first.begin) {
+            // We have a gap to infill (we supply pos for ("insert in front of" calls)
+            ops.infill(map, pos, KeyType(current_begin, std::min(range.end, pos->first.begin)));
+            // Advance current begin, but *not* pos as it's the next valid value. (infill shall not invalidate pos)
+            current_begin = pos->first.begin;
+        } else {
+            // We need to run the update operation on the valid portion of the current value
+            if (pos->first.end > range.end) {
+                // If this entry overlaps end-of-range we need to trim it to the range
+                pos = map.split(pos, range.end, sparse_container::split_op_keep_both());
+            }
+
+            // We have a valid fully contained range, merge with it
+            ops.update(pos);
+
+            // Advance the current location and map entry
+            current_begin = pos->first.end;
+            ++pos;
+        }
+    }
+
+    // Fill to the end as needed
+    if (current_begin < range.end) {
+        ops.infill(map, pos, KeyType(current_begin, range.end));
+    }
+}
+
 // Parallel iterator
 // Traverse to range maps over the the same range, but without assumptions of aligned ranges.
 // ++ increments to the next point where on of the two maps changes range, giving a range over which the two

layers/sync/sync_validation.cpp

@@ -1651,47 +1651,35 @@ SyncStageAccessFlags SyncStageAccess::AccessScope(VkPipelineStageFlags2KHR stage
     return AccessScopeByStage(stages) & AccessScopeByAccess(accesses);
 }
 
+// The semantics of the InfillUpdateOps of infill_update_range are slightly different than for the UpdateMemoryAccessState Action
+// operations, as this simplifies the generic traversal.  So we wrap them in a semantics Adapter to get the same effect.
 template <typename Action>
-void UpdateMemoryAccessState(ResourceAccessRangeMap *accesses, const ResourceAccessRange &range, const Action &action) {
-    // TODO: Optimization for operations that do a pure overwrite (i.e. WRITE usages which rewrite the state, vs READ usages
-    //       that do incrementalupdates
-    assert(accesses);
-    if (range.empty()) return;
-    auto pos = accesses->lower_bound(range);
-    if (pos == accesses->end() || !pos->first.intersects(range)) {
-        // The range is empty, fill it with a default value.
-        pos = action.Infill(accesses, pos, range);
-    } else if (range.begin < pos->first.begin) {
-        // Leading empty space, infill
-        pos = action.Infill(accesses, pos, ResourceAccessRange(range.begin, pos->first.begin));
-    } else if (pos->first.begin < range.begin) {
-        // Trim the beginning if needed
-        pos = accesses->split(pos, range.begin, sparse_container::split_op_keep_both());
-        ++pos;
-    }
-
-    const auto the_end = accesses->end();
-    while ((pos != the_end) && pos->first.intersects(range)) {
-        if (pos->first.end > range.end) {
-            pos = accesses->split(pos, range.end, sparse_container::split_op_keep_both());
-        }
-
-        pos = action(accesses, pos);
-        if (pos == the_end) break;
-
-        auto next = pos;
-        ++next;
+struct ActionToOpsAdapter {
+    using Map = ResourceAccessRangeMap;
+    using Range = typename Map::key_type;
+    using Iterator = typename Map::iterator;
+    using IndexType = typename Map::index_type;
+
+    void infill(Map &accesses, const Iterator &pos, const Range &infill_range) const {
+        // Combine Infill and update operations to make the generic implementation simpler
+        Iterator infill = action.Infill(&accesses, pos, infill_range);
+        if (infill == accesses.end()) return;  // Allow action to 'pass' on filling in the blanks
+
+        // Need to apply the action to the Infill.  'infill_update_range' expect ops.infill to be completely done with
+        // the infill_range, where as Action::Infill assumes the caller will apply the action() logic to the infill_range
+        for (; infill != pos; ++infill) {
+            assert(infill != accesses.end());
+            action(infill);
+        }
+    }
+    void update(const Iterator &pos) const { action(pos); }
+    const Action &action;
+};
 
-        // Do gap infill or infill to end of range, if needed.
-        if (pos->first.end < range.end) {
-            VkDeviceSize limit = (next == the_end) ? range.end : std::min(range.end, next->first.begin);
-            ResourceAccessRange new_range(pos->first.end, limit);
-            if (new_range.non_empty()) {
-                next = action.Infill(accesses, next, new_range);
-            }
-        }
-        pos = next;
-    }
+template <typename Action>
+void UpdateMemoryAccessState(ResourceAccessRangeMap *accesses, const ResourceAccessRange &range, const Action &action) {
+    ActionToOpsAdapter<Action> ops{action};
+    infill_update_range(*accesses, range, ops);
 }
 
 // Give a comparable interface for range generators and ranges
@@ -1726,10 +1714,9 @@ struct UpdateMemoryAccessStateFunctor {
         return accesses->lower_bound(range);
     }
 
-    Iterator operator()(ResourceAccessRangeMap *accesses, const Iterator &pos) const {
+    void operator()(const Iterator &pos) const {
         auto &access_state = pos->second;
         access_state.Update(usage, ordering_rule, tag);
-        return pos;
     }
 
     UpdateMemoryAccessStateFunctor(AccessAddressType type_, const AccessContext &context_, SyncStageAccessIndex usage_,
@@ -1801,7 +1788,7 @@ class ApplyBarrierOpsFunctor {
         return inserted;
     }
 
-    Iterator operator()(ResourceAccessRangeMap *accesses, const Iterator &pos) const {
+    void operator()(const Iterator &pos) const {
         auto &access_state = pos->second;
         for (const auto &op : barrier_ops_) {
             op(&access_state);
@@ -1812,7 +1799,6 @@ class ApplyBarrierOpsFunctor {
             // another walk
             access_state.ApplyPendingBarriers(tag_);
         }
-        return pos;
     }
 
     // A valid tag is required IFF layout_transition is true, as transitions are write ops