fix(agc): avoid redundant transfers for untracked imported textures

This commit is contained in:
boykopovar 2026-10-09 10:50:49 +03:00
parent a8b032b236
commit 0e1d25a8a2
6 changed files with 145 additions and 10 deletions

View File

@ -92,6 +92,7 @@ bool StoredOver(std::uint64_t address, std::size_t bytes, std::uint64_t generati
// for every collect: nothing is reused without an ordering point. APS5_PACKET_EPOCH=1 makes the
// workers bump before every packet as before. CollectEpochBumps counts the bumps ([guestmem] line).
void BumpCollectEpoch();
std::uint64_t CollectEpoch();
std::uint64_t CollectEpochBumps();
std::uint64_t CollectWritesUncached(std::uint64_t address, std::size_t bytes);
// The tracker's current generation (every collect and MarkWritten bumps it): a stamp taken after

View File

@ -1139,6 +1139,10 @@ std::uint64_t CollectEpochBumps() {
return collectEpochBumps.load(std::memory_order_relaxed);
}
std::uint64_t CollectEpoch() {
return collectMemoEnabled() ? threadCollectEpoch : 0;
}
namespace {
void unwatchLocked(WriteTracker& tracker, std::uint64_t address, std::size_t bytes) {
@ -1225,8 +1229,9 @@ bool UnchangedSinceAll(std::span<const UnchangedQuery> queries) {
std::uint64_t storeOwn(std::uint64_t address, std::size_t bytes, const std::function<std::pair<std::uint64_t, std::uint64_t>()>& store) {
if (bytes == 0) return 0;
const auto stampStored = [](WriteTracker& tracker, std::pair<std::uint64_t, std::uint64_t> stored) -> std::uint64_t {
if (stored.second <= stored.first || !tracker.watched || !tracker.covers(stored.first, static_cast<std::size_t>(stored.second - stored.first))) return 0;
if (stored.second <= stored.first) return 0;
++tracker.generation;
if (!tracker.watched || !tracker.covers(stored.first, static_cast<std::size_t>(stored.second - stored.first))) return 0;
for (auto block = tracker.blockOf(stored.first); block <= tracker.blockOf(stored.second - 1); ++block) {
tracker.stamp(block, tracker.generation, StampKind::Driver);
tracker.noteDriverStore(block, stored.first, stored.second, tracker.generation);
@ -1260,10 +1265,11 @@ std::uint64_t MarkWritten(std::uint64_t address, std::size_t bytes) {
auto& tracker = Tracker();
const auto lock = lockTracker(tracker);
tracker.initialize();
if (!tracker.watched || bytes == 0 || !tracker.covers(address, bytes)) return 0;
if (bytes == 0) return 0;
++tracker.generation;
if (!tracker.watched || !tracker.covers(address, bytes)) return 0;
const auto first = tracker.blockOf(address);
const auto last = tracker.blockOf(address + bytes - 1);
++tracker.generation;
for (auto block = first; block <= last; ++block) {
tracker.stamp(block, tracker.generation, StampKind::Driver);
tracker.noteDriverStore(block, address, address + bytes, tracker.generation);

View File

@ -297,6 +297,8 @@ private:
// Uploads the surface, or only the tracked layers `layers` selects (the direct path; the others
// upload everything).
void upload(const std::vector<bool>* layers = nullptr);
void captureGuestBytes(const std::vector<bool>* layers);
bool compareUntracked(std::uint64_t address, std::size_t bytes, std::span<std::uint8_t> changed, bool memoize = false) const;
// Stores the pending tracked layers overlapping [address, address + bytes) to guest memory; in
// each, 64 KiB blocks the CPU wrote since the layer's generation keep the CPU's bytes. Block
// units asked for in pieces too often are all stored at once for a while (the hysteresis:
@ -425,6 +427,7 @@ private:
std::uint64_t sliceLinearBytes = 0;
SurfaceGeometry geometry;
std::vector<std::byte> original;
mutable std::array<std::uint64_t, 4> comparedGuestBytes{};
// DCC keys the image content was uploaded under: a fast-cleared surface starts as its clear value.
DccKeys uploadedKeys = DccKeys::Uncompressed;
mutable DccKeys filledKeys = DccKeys::Uncompressed;

View File

@ -1239,6 +1239,7 @@ bool StorageTexture::Refresh() {
bool unchanged = true;
bool stamped = true;
bool tracked = true;
bool compared = false;
bool keysChanged = false;
bool cpuWrote = false;
bool direct = false;
@ -1254,6 +1255,8 @@ bool StorageTexture::Refresh() {
stampedBlocks.assign(generations.size(), 0);
cpuBlocks.assign(generations.size(), 0);
tracked = GuestMemory::ChangedBlocks(descriptor.baseAddress, static_cast<std::size_t>(guestBytes), generations, stampedBlocks, cpuBlocks);
compared = !tracked && compareUntracked(descriptor.baseAddress, static_cast<std::size_t>(guestBytes), stampedBlocks, true);
if (compared) cpuBlocks = stampedBlocks;
};
const auto keysStart = profile ? std::chrono::steady_clock::now() : std::chrono::steady_clock::time_point{};
keys = ProvedKeys();
@ -1289,8 +1292,16 @@ bool StorageTexture::Refresh() {
}
// "cpu" names a CPU store in both models; a block without a generation (untracked) says
// nothing about who wrote it.
if (compared && !keysChanged) {
for (std::uint32_t layer = 0; layer < trackedLayers; ++layer) {
const auto first = layerBegin(layer) / 65536 - descriptor.baseAddress / 65536;
const auto last = (layerBegin(layer) + layerBytes(layer) - 1) / 65536 - descriptor.baseAddress / 65536;
changed[layer] = std::any_of(stampedBlocks.begin() + first, stampedBlocks.begin() + last + 1, [](auto value) { return value != GuestMemory::BlockUnchanged; });
}
unchanged = std::none_of(changed.begin(), changed.end(), [](bool value) { return value; });
}
for (std::size_t k = 0; k < cpuBlocks.size(); ++k) {
if (tracked && cpuBlocks[k] != 0 && generations[k] != 0) cpuWrote = true;
if (cpuBlocks[k] != 0 && (compared || (tracked && generations[k] != 0))) cpuWrote = true;
}
// Unchanged bytes rescue an upload only when nothing else moved: with the keys changed
// the same bytes read differently, and a changed unit holding pending results must take
@ -1360,6 +1371,7 @@ bool StorageTexture::Refresh() {
// unit's stamps say nothing, so it is stored.
const auto droppable = [&](std::uint32_t unit) {
if (keysChanged && IsDccClear(keys)) return true;
if (compared) return stampedBlocks.at(unit) == GuestMemory::BlockWritten;
if (!tracked || layerGeneration[unit] == 0 || unit >= stampedBlocks.size() || stampedBlocks[unit] != GuestMemory::BlockWritten) return false;
const auto begin = layerBegin(unit);
const auto bytes = layerBytes(unit);
@ -1477,6 +1489,55 @@ void traceKeyStore(const char* path, const GuestTextureResource& descriptor, std
}
void StorageTexture::captureGuestBytes(const std::vector<bool>* layers) {
comparedGuestBytes = {};
const bool complete = layers == nullptr || std::all_of(layers->begin(), layers->end(), [](bool selected) { return selected; });
for (std::uint32_t layer = 0; layer < trackedLayers; ++layer) {
if (layers != nullptr && !(*layers)[layer]) continue;
const auto offset = static_cast<std::size_t>(layerBegin(layer) - descriptor.baseAddress);
GuestMemory::ReadCommitted(layerBegin(layer), std::span(original).subspan(offset, static_cast<std::size_t>(layerBytes(layer))));
}
originalValid = originalValid || complete;
}
bool StorageTexture::compareUntracked(std::uint64_t address, std::size_t bytes, std::span<std::uint8_t> changed, bool memoize) const {
if (!originalValid) return false;
struct Exempt {
const StorageTexture* previous;
~Exempt() { refreshing = previous; }
} exempt{refreshing};
refreshing = this;
Require(address >= descriptor.baseAddress && bytes <= guestBytes && address - descriptor.baseAddress <= guestBytes - bytes, "texture comparison exceeds its snapshot");
Require(original.size() == guestBytes, "texture comparison has no complete snapshot");
GuestMemory::FlushGpuWrites(address, bytes);
if (!originalValid) return false;
constexpr std::uint64_t blockBytes = 65536;
const auto end = address + bytes;
const auto first = address / blockBytes;
Require(changed.size() == (end - 1) / blockBytes - first + 1, "texture comparison block count differs");
const auto* recorder = Recorder::Active();
const std::array<std::uint64_t, 4> stamp{GuestMemory::CollectEpoch(), GuestMemory::TrackerGeneration(), GuestMemory::ForgetSerial(), recorder != nullptr ? recorder->NewestWriteNote(address, bytes) : 0};
const bool cacheable = memoize && stamp[0] != 0 && (stamp[2] & 1u) == 0 && address == descriptor.baseAddress && bytes == guestBytes;
if (cacheable && comparedGuestBytes == stamp) {
std::fill(changed.begin(), changed.end(), GuestMemory::BlockUnchanged);
return true;
}
const auto saved = std::span(original).subspan(static_cast<std::size_t>(address - descriptor.baseAddress), bytes);
if (GuestMemory::EqualsCommittedUnsynced(address, saved)) {
std::fill(changed.begin(), changed.end(), GuestMemory::BlockUnchanged);
if (cacheable) comparedGuestBytes = stamp;
return true;
}
comparedGuestBytes = {};
for (std::size_t index = 0; index < changed.size(); ++index) {
const auto begin = std::max(address, (first + index) * blockBytes);
const auto stop = std::min(end, (first + index + 1) * blockBytes);
const auto saved = std::span(original).subspan(static_cast<std::size_t>(begin - descriptor.baseAddress), static_cast<std::size_t>(stop - begin));
changed[index] = GuestMemory::EqualsCommittedUnsynced(begin, saved) ? GuestMemory::BlockUnchanged : GuestMemory::BlockWritten;
}
return true;
}
void StorageTexture::upload(const std::vector<bool>* layers) {
CaptureTrace::Log("upload image=%llx bytes=%llu generation=%llu reason=%s partial=%d", static_cast<unsigned long long>(descriptor.baseAddress), static_cast<unsigned long long>(guestBytes), static_cast<unsigned long long>(generation), uploadReason, layers != nullptr);
const bool profile = LookupOutcomes::Profiled();
@ -1502,7 +1563,7 @@ void StorageTexture::upload(const std::vector<bool>* layers) {
// The clear is recorded into the open batch like a direct upload (the image kept by it): a
// batch of its own submitted the recorder's work first and waited for all of it, 25-40 ms
// under the GPU mutex at the movie stage. APS5_NO_RECORDED_CLEAR=1 waits as before.
originalValid = false;
captureGuestBytes(nullptr);
forgetBorrowed(0, trackedLayers);
stampLayers(false);
static const bool recordClear = std::getenv("APS5_NO_RECORDED_CLEAR") == nullptr;
@ -1550,9 +1611,8 @@ void StorageTexture::upload(const std::vector<bool>* layers) {
return;
}
if (const auto* import = uploadedKeys == DccKeys::Uncompressed ? HostImportFor(context, descriptor.baseAddress, static_cast<std::size_t>(guestBytes)) : nullptr) {
// The surface lives in host-imported memory: the detiler reads it in place, no guest bytes
// are copied, and write tracking alone validates the image (a change re-runs this).
originalValid = false;
if (GuestMemory::Watched(descriptor.baseAddress, static_cast<std::size_t>(guestBytes))) originalValid = false;
else captureGuestBytes(layers);
stampLayers(true);
// A whole-surface upload of a block-unit image goes through the windows too when a unit
// shadow holds part of the surface (the detile then reads the slabs; a new image has no
@ -3407,11 +3467,12 @@ void StorageTexture::writeBackLayers(const std::vector<bool>& layers) {
}
std::vector<std::uint8_t> changedBlocks(spanBlocks);
const bool tracked = GuestMemory::ChangedBlocks(firstStored, static_cast<std::size_t>(lastStored - firstStored), generations, changedBlocks);
const bool compared = !tracked && compareUntracked(firstStored, static_cast<std::size_t>(lastStored - firstStored), changedBlocks);
for (std::uint32_t layer = 0; layer < trackedLayers; ++layer) {
if (!layers[layer]) continue;
const auto begin = layerBegin(layer);
const auto end = begin + layerBytes(layer);
if (!tracked || layerGeneration[layer] == 0) {
if (!compared && (!tracked || layerGeneration[layer] == 0)) {
keep.emplace_back(begin, end);
continue;
}
@ -3420,7 +3481,7 @@ void StorageTexture::writeBackLayers(const std::vector<bool>& layers) {
const auto to = std::min(at + block, end);
if (from >= to) continue;
const bool edge = from != at || to != at + block;
if (changedBlocks[static_cast<std::size_t>((at - spanBegin) / block)] == GuestMemory::BlockWritten && (!edge || GuestMemory::StoredOver(from, static_cast<std::size_t>(to - from), layerGeneration[layer]))) {
if (changedBlocks[static_cast<std::size_t>((at - spanBegin) / block)] == GuestMemory::BlockWritten && (compared || !edge || GuestMemory::StoredOver(from, static_cast<std::size_t>(to - from), layerGeneration[layer]))) {
skippedAny = true;
skippedLayer[layer] = true;
++skipped;

View File

@ -130,7 +130,12 @@ void CheckUnwatch() {
Require(!Watched(imported, Block) && !UnchangedSince(imported, Block, before), "unwatch kept trusting import stamps");
Require(CollectWrites(base, 3 * Block) == 0 && CollectWritesUncached(imported, Block) == 0, "unwatch did not invalidate a cached collect");
Require(Watched(base, Block) && Watched(base + 2 * Block, Block), "unwatch disabled unrelated memory");
const auto beforeDriverWrite = TrackerGeneration();
Require(MarkWritten(imported, Block) == 0, "an unwatched import still produces trusted stamps");
Require(TrackerGeneration() > beforeDriverWrite, "an unwatched driver write did not invalidate byte comparisons");
const auto beforeOwnWrite = TrackerGeneration();
StoreOwnBytes(imported, 1, [&] { *reinterpret_cast<std::uint8_t*>(imported) = 0x42; });
Require(TrackerGeneration() > beforeOwnWrite, "an unwatched CPU store did not invalidate byte comparisons");
std::array<std::uint64_t, 1> generations{before};
std::array<std::uint8_t, 1> changed{};
Require(!ChangedBlocks(imported, Block, generations, changed), "an unwatched import still uses block stamps");

View File

@ -201,6 +201,12 @@ void Run(AgcDriver::VulkanDevice& device, std::uint8_t* block) {
Require(image != nullptr && image->Descriptor().dccAddress == AddressOf(keys), "the written surface has no pending image under its keys");
Require(AgcDriver::Graphics::StorageImageServesKeys(*image, AddressOf(copied)), "the written image does not serve the copied keys");
Require(image->UploadedKeys() == DccKeys::Uncompressed && image->FilledKeys() == DccKeys::Uncompressed, "the written image still holds the clear it was uploaded under");
const auto pendingVersion = image->Version();
for (int repeat = 0; repeat < 3; ++repeat) {
Require(image->Refresh(), "an unchanged pending image was uploaded again");
Require(image->Version() == pendingVersion, "an unchanged pending image changed version");
Require(StorageTexture::FindPending(surface, SurfaceBytes) == image, "an unchanged refresh wrote the pending image back");
}
StorageTexture::FlushPending(surface, SurfaceBytes, nullptr, "test");
device.WaitIdle();
RequireKeys(keys, 0xff, "the keys after the write-back");
@ -329,16 +335,69 @@ void Run(AgcDriver::VulkanDevice& device, std::uint8_t* block) {
RequireKeys(keys, 0x40, "a write marked after a 0001 key fill over a cleared image, before a refresh saw the fill");
}
void RunUntrackedReuse(AgcDriver::VulkanDevice& device, std::uint8_t* block) {
const auto address = AddressOf(block);
std::memset(block, 0x55, BlockBytes);
auto descriptor = TextureDescriptor(block, nullptr);
descriptor[2] = ((Side - 1u) >> 2u) | ((2 * Side - 1u) << 14u);
descriptor[6] = 0;
descriptor[7] = 0;
std::vector<std::uint32_t> userData(16, 0u);
std::copy(descriptor.begin(), descriptor.end(), userData.begin() + 4);
std::lock_guard gpu(AgcDriver::GuestMemory::GpuMutex());
Dispatch(device, WriteCode, userData);
const auto image = StorageTexture::FindPending(address, BlockBytes);
Require(image != nullptr && image->GuestBytes() == BlockBytes, "the two-block image has no pending results");
Require(!AgcDriver::GuestMemory::Watched(address, BlockBytes), "the imported test image is still write-watched");
AgcDriver::GuestMemory::BumpCollectEpoch();
const auto version = image->Version();
for (int repeat = 0; repeat < 3; ++repeat) {
Require(image->Refresh() && image->Version() == version, "an unchanged untracked image was uploaded again");
Require(Count(block, BlockBytes, 0x55) == BlockBytes, "refresh wrote GPU results into unchanged guest memory");
}
std::memset(block + SurfaceBytes, 0x33, SurfaceBytes);
AgcDriver::GuestMemory::BumpCollectEpoch();
Require(!image->Refresh(), "an untracked CPU edit was missed");
const auto editedVersion = image->Version();
Require(image->Refresh() && image->Version() == editedVersion, "the CPU-edited image was uploaded twice");
StorageTexture::FlushPending(address, BlockBytes, nullptr, "test untracked reuse");
device.WaitIdle();
Require(Count(block, SurfaceBytes, 0xff) == Threads * 4u, "refresh lost the unchanged block's GPU results");
Require(Count(block, SurfaceBytes, 0x55) == SurfaceBytes - Threads * 4u, "refresh changed unrelated texels");
Require(Count(block + SurfaceBytes, SurfaceBytes, 0x33) == SurfaceBytes, "write-back overwrote the CPU-edited block");
Dispatch(device, WriteCode, userData);
Require(image->Refresh(), "a freshly dispatched image did not retain its snapshot");
std::memset(block, 0x77, SurfaceBytes);
image->WriteBack();
device.WaitIdle();
Require(Count(block, SurfaceBytes, 0x77) == SurfaceBytes, "write-back without refresh overwrote a CPU edit");
image->Refresh();
Require(image->Refresh(), "the image did not stabilize after write-back");
AgcDriver::GuestMemory::StoreOwnBytes(address, SurfaceBytes, [&] { std::memset(block, 0x22, SurfaceBytes); });
Require(!image->Refresh(), "a driver store within the comparison epoch was missed");
Require(image->Refresh(), "the driver-edited image was uploaded twice");
const std::array<std::uint32_t, 4> pattern{0x44444444u, 0x44444444u, 0x44444444u, 0x44444444u};
Require(device.FillBuffer(address, SurfaceBytes, pattern), "the imported GPU fill was not recorded");
Require(!image->Refresh(), "a pending GPU store within the comparison epoch was missed");
device.WaitIdle();
Require(Count(block, SurfaceBytes, 0x44) == SurfaceBytes, "the GPU fill did not reach guest memory");
Require(image->Refresh(), "the GPU-edited image was uploaded twice");
}
}
int main() {
try {
std::optional<GuestBlock> block;
std::optional<GuestBlock> watched;
std::optional<GuestBlock> untracked;
const auto device = OpenVulkanTestDevice();
if (!device) return VulkanTestSkipped;
block.emplace(false);
Run(*device, block->Data());
untracked.emplace(false);
RunUntrackedReuse(*device, untracked->Data());
if (AgcDriver::GuestMemory::WriteWatched()) {
watched.emplace(true);
Run(*device, watched->Data());