Skip clean memory tracker ranges during buffer synchronization

src/graphics/host_gpu/regionManager.h

@@ -114,6 +114,9 @@ public:
 	void ForEachModifiedRange(uint64_t vaddr, uint64_t size, Func&& func) {
 		const auto [start, end] = GetPageRange(vaddr, size);
 		auto&      bits         = GetBits<source>();
+		if (bits.FirstRangeFrom(start).first >= end) {
+			return;
+		}
 		RegionBits mask(bits, start, end);
 		if constexpr (clear) {
 			bits.UnsetRange(start, end);

tests/MemoryTrackerTests.cpp

@@ -402,6 +402,100 @@ void TestCpuDirtyUpload() {
   Release(memory);
 }
 
+void TestCleanUploadPreservesOwnership() {
+  constexpr auto page_size = Libs::Graphics::TRACKER_PAGE_SIZE;
+  constexpr auto region_size = Libs::Graphics::TRACKER_REGION_SIZE;
+  TrackerHarness harness;
+  auto &tracker = harness.tracker;
+  auto *memory = AllocateFixedGuestRange(region_size * 2, region_size);
+  const auto address = reinterpret_cast<uint64_t>(memory);
+  tracker.ForEachUploadRange(address, region_size * 2, false,
+                             [](uint64_t, uint64_t) noexcept {},
+                             []() noexcept {});
+
+  for (const auto start : {address + 63 * page_size,
+                           address + region_size - page_size}) {
+    const auto before = start - page_size;
+    const auto after = start + 2 * page_size;
+    tracker.MarkRegionAsCpuModified(before, page_size);
+    tracker.MarkRegionAsCpuModified(after, page_size);
+    uint32_t ranges = 0;
+    uint32_t completions = 0;
+    ResetProtectionLog();
+    tracker.ForEachUploadRange(
+        start, 2 * page_size, false,
+        [&](uint64_t, uint64_t) noexcept { ranges++; },
+        [&]() noexcept { completions++; });
+    Check(ranges == 0 && completions == 1 && g_protection_calls == 0 &&
+              !tracker.IsRegionCpuModified(start, 2 * page_size),
+          "clean read-only upload changed dirty state or protection");
+
+    tracker.ForEachUploadRange(
+        start, 2 * page_size, true,
+        [&](uint64_t, uint64_t) noexcept { ranges++; },
+        [&]() noexcept { completions++; });
+    Check(ranges == 0 && completions == 2 &&
+              tracker.IsRegionGpuModified(start, page_size) &&
+              tracker.IsRegionGpuModified(start + page_size, page_size) &&
+              Protection(reinterpret_cast<void *>(start)) == PAGE_NOACCESS &&
+              Protection(reinterpret_cast<void *>(start + page_size)) ==
+                  PAGE_NOACCESS &&
+              tracker.IsRegionCpuModified(before, page_size) &&
+              tracker.IsRegionCpuModified(after, page_size) &&
+              !tracker.IsRegionGpuModified(before, page_size) &&
+              !tracker.IsRegionGpuModified(after, page_size) &&
+              IsWritable(reinterpret_cast<void *>(before)) &&
+              IsWritable(reinterpret_cast<void *>(after)),
+          "clean written upload lost GPU ownership or changed dirty neighbors");
+    tracker.UnmarkRegionAsGpuModified(start, 2 * page_size);
+  }
+  tracker.UntrackMemory(address, region_size * 2);
+  Release(memory);
+}
+
+void BenchmarkCleanUploads() {
+  constexpr uint64_t size = 256ull * 1024 * 1024;
+  TrackerHarness harness;
+  auto &tracker = harness.tracker;
+  auto *memory = AllocateFixedGuestRange(size, 0);
+  const auto address = reinterpret_cast<uint64_t>(memory);
+  tracker.ForEachUploadRange(address, size, false,
+                             [](uint64_t, uint64_t) noexcept {},
+                             []() noexcept {});
+  std::puts("chunk_bytes,sweeps,calls,elapsed_ns,ns_per_sweep,ns_per_call");
+  for (const uint64_t chunk : {size, Libs::Graphics::TRACKER_REGION_SIZE,
+                               uint64_t{64 * 1024}}) {
+    uint64_t ranges = 0;
+    uint64_t completions = 0;
+    uint64_t sweeps = 0;
+    ResetProtectionLog();
+    const auto start = std::chrono::steady_clock::now();
+    std::chrono::nanoseconds elapsed{};
+    do {
+      for (uint64_t offset = 0; offset < size; offset += chunk) {
+        tracker.ForEachUploadRange(
+            address + offset, chunk, false,
+            [&](uint64_t, uint64_t) noexcept { ranges++; },
+            [&]() noexcept { completions++; });
+      }
+      sweeps++;
+      elapsed = std::chrono::steady_clock::now() - start;
+    } while (elapsed < std::chrono::milliseconds(250));
+    Check(ranges == 0 && completions == sweeps * (size / chunk) &&
+              g_protection_calls == 0,
+          "clean upload benchmark changed ranges, completion, or protection");
+    std::printf("%llu,%llu,%llu,%lld,%.2f,%.2f\n",
+                static_cast<unsigned long long>(chunk),
+                static_cast<unsigned long long>(sweeps),
+                static_cast<unsigned long long>(completions),
+                static_cast<long long>(elapsed.count()),
+                static_cast<double>(elapsed.count()) / sweeps,
+                static_cast<double>(elapsed.count()) / completions);
+  }
+  tracker.UntrackMemory(address, size);
+  Release(memory);
+}
+
 void TestRangeInvalidation() {
   TrackerHarness harness;
   auto &tracker = harness.tracker;
@@ -1027,11 +1121,16 @@ int main(int argc, char **argv) {
   if (argc == 3 && std::strcmp(argv[1], "--death") == 0) {
     RunDeathCase(argv[2]);
   }
+  if (argc == 2 && std::strcmp(argv[1], "--benchmark-clean-upload") == 0) {
+    BenchmarkCleanUploads();
+    return 0;
+  }
   TestGuestRange();
   TestRangeSet();
   TestQueriesDoNotRequireMappedOwnership();
   TestConcurrentRegionPublication();
   TestCpuDirtyUpload();
+  TestCleanUploadPreservesOwnership();
   TestRangeInvalidation();
   TestGpuReacquisitionAfterInvalidation();
   TestGpuDirtyBits();