Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 83 additions & 35 deletions gpu/vk/vk_memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,17 +348,20 @@ bool import_pool(uint64_t lo, uint64_t hi, Pool& p) {
ms(b, c), ms(c, d), ms(d, e));
}
} report{t0, t1, t2, t3, t4};
// (pages the game has not touched yet may be only reserved: committed now, as the pool's memory is usable; a range
// committed already is left alone: the call took milliseconds in the game's huge reservation)
{
// (pages the game has not touched yet may be only reserved: committed now, as the pool's memory is usable)
// (hookshot) In one call over the range: the guest window is one reservation, committing pages committed already

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Small correction to the comment: I'm fairly sure VirtualAlloc(MEM_COMMIT) over pages that are already committed doesn't fail, but it does apply the new protection to them. So they don't stay exactly as they were. Today that only affects RX_NVN_WATCH_*/hos_watch_guest_writes: an import over a watched range silently disarms it until the next 2 ms re-arm. That's fine for a diagnostic, but it would be good to say so here so nobody later relies on VirtualProtect for something that isn't a diagnostic. Also, this comment says ~33 of ~70 ms and 1.3 ms, while the PR text says ~46 of ~95 ms and ~3 ms. Worth making them match.

// leaves them as they are, and the guest's pages are all read-write (only the RX_NVN_WATCH_* diagnostics protect
// some). The pages were queried one region at a time before, so as to commit only the reserved ones: the queries were
// ~33 ms of an import's ~70 ms in the game's huge reservation (on the replay thread, at a pool's first binding), the
// call 1.3 ms. Where it fails, as before
if (VirtualAlloc(ptr, (SIZE_T)bytes, MEM_COMMIT, PAGE_READWRITE) != ptr)
for (uint8_t* q = ptr; q < ptr + bytes;) {
MEMORY_BASIC_INFORMATION mi{};
if (VirtualQuery(q, &mi, sizeof mi) != sizeof mi) break;
uint8_t* end = std::min((uint8_t*)mi.BaseAddress + mi.RegionSize, ptr + bytes);
if (mi.State == MEM_RESERVE) VirtualAlloc(q, (size_t)(end - q), MEM_COMMIT, PAGE_READWRITE);
q = end;
}
}
#ifdef __linux__
if (import_dma_buf(lo, hi, p)) {
t1 = t2 = t3 = t4 = clk::now();
Expand Down Expand Up @@ -1756,6 +1759,41 @@ void prefetch_chunks(uint64_t lo, uint64_t hi, uint64_t chunk) {
bg_cv.notify_one();
}

/// (imports by 16 MB chunk (RX_VK_IMPORT_CHUNK_MB), not by pool: each import cost 5-45 ms in the driver, more as their
/// number grew, and the game makes ~13 pools a second; a chunk covers the pools after it at no cost)
uint64_t import_chunk() {
static const uint64_t chunk = (uint64_t)(getenv("RX_VK_IMPORT_CHUNK_MB") ? std::max(0, atoi(getenv("RX_VK_IMPORT_CHUNK_MB"))) : 16) << 20;
return chunk;
}
/// (hookshot) A game pool across a chunk's end had each chunk imported on its own, at the first binding in it, and a later
/// binding across the end imported the two again as one, on the replay thread: OoT's 16 MB pool at 0x1021745000 bound at
/// 0x1021ffff00+0x160 across 0x1022000000, 130-147 ms, the one hitch of a panning run in Kokiri. Up to this size an import
/// takes the chunks of the game pools across its ends too (RX_VK_IMPORT_SPAN_MB, default 64; 0: not)
uint64_t import_span() {
static const uint64_t span = (uint64_t)(getenv("RX_VK_IMPORT_SPAN_MB") ? std::max(0, atoi(getenv("RX_VK_IMPORT_SPAN_MB"))) : 64) << 20;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is on for every platform and vendor, but it's only been measured on Windows + NVIDIA (see the review body). If you can't test Linux/AMD, a platform-dependent default would be the safe choice. Otherwise, a line in the comment that it applies everywhere would help.

return span;
}
/// [lo, hi), chunk-aligned, grown over the game pools that run across its ends, while it stays within import_span (pools_mu held)
void span_game_pools(uint64_t& lo, uint64_t& hi, uint64_t chunk) {
const uint64_t span = import_span();
if (!chunk || hi - lo >= span) return;
for (bool grew = true; grew;) {
grew = false;
for (const uint64_t e : {lo, hi}) {
// (a pool across e that fits starts above e - span: the pools from there to e, as the host's may overlap)
auto g = std::upper_bound(guest_pools.begin(), guest_pools.end(), e, [](uint64_t v, const GuestPool& p) { return v < p.lo; });
while (g != guest_pools.begin()) {
--g;
if (g->lo + span <= e) break;
if (g->lo >= e || g->hi <= e) continue;
const uint64_t nlo = std::min(lo, g->lo / chunk * chunk), nhi = std::max(hi, (g->hi + chunk - 1) / chunk * chunk);
if (nhi - nlo > span || (nlo == lo && nhi == hi)) continue;
lo = nlo, hi = nhi, grew = true;
}
}
}
}

Pool* pool_lookup(uint64_t guest, uint64_t size) {
if (guest_pools.empty()) return nullptr;
bg_drain();
Expand All @@ -1764,9 +1802,7 @@ Pool* pool_lookup(uint64_t guest, uint64_t size) {
if (it == guest_pools.begin()) return nullptr;
--it;
if (guest < it->lo || guest >= it->hi || !size || size > it->hi - guest) return nullptr;
// (imports by 16 MB chunk (RX_VK_IMPORT_CHUNK_MB), not by pool: each import cost 5-45 ms in the driver, more as
// their number grew, and the game makes ~13 pools a second; a chunk covers the pools after it at no cost)
static const uint64_t chunk = (uint64_t)(getenv("RX_VK_IMPORT_CHUNK_MB") ? std::max(0, atoi(getenv("RX_VK_IMPORT_CHUNK_MB"))) : 16) << 20;
const uint64_t chunk = import_chunk();
const GuestPool range = chunk ? GuestPool{guest, guest + size} : *it;
auto covering = [&]() {
auto c = imported.upper_bound(range.lo);
Expand Down Expand Up @@ -1806,43 +1842,55 @@ Pool* pool_lookup(uint64_t guest, uint64_t size) {
// pool stayed: two mirrors of the same memory. MKW's GPU culling wrote its instance data (a 2.4 MB storage
// buffer) through one, the draws read it through the other, synced from guest memory before the submission's
// write-back: the instances of a frame before or older, objects popping in and out, one-frame flashes.)
uint64_t lo = want_lo, hi = std::max(want_hi, imp != imported.end() ? imp->second.hi : 0);
uint64_t lo = 0, hi = 0;
std::vector<uint64_t> overlapped;
for (bool grew = true; grew;) {
grew = false;
overlapped.clear();
auto it = imported.lower_bound(hi); // (the pools starting below hi, the last ones first)
while (it != imported.begin()) {
--it;
if (it->second.hi <= lo) {
if (it->second.hi + (1ull << 30) < lo) break; // (imports are at most 1 GB: none further down reaches lo)
continue;
auto plan = [&](bool spanned) { // (the range, the imported pools it takes in; spanned: span_game_pools's chunks too)
lo = want_lo, hi = std::max(want_hi, imp != imported.end() ? imp->second.hi : 0);
if (spanned && chunk) span_game_pools(lo, hi, chunk);
for (bool grew = true; grew;) {
grew = false;
overlapped.clear();
auto it = imported.lower_bound(hi); // (the pools starting below hi, the last ones first)
while (it != imported.begin()) {
--it;
if (it->second.hi <= lo) {
if (it->second.hi + (1ull << 30) < lo) break; // (imports are at most 1 GB: none further down reaches lo)
continue;
}
overlapped.push_back(it->first);
if (it->second.lo < lo) lo = it->second.lo, grew = true;
if (it->second.hi > hi) hi = it->second.hi, grew = true;
}
overlapped.push_back(it->first);
if (it->second.lo < lo) lo = it->second.lo, grew = true;
if (it->second.hi > hi) hi = it->second.hi, grew = true;
}
}
if (hi - lo > (1ull << 30)) { // (no larger than 1 GB: then as before, the chunk's pool alone, overlaps logged)
static int logged_big;
if (logged_big++ < 8)
logf("vk: pool %#llx..%#llx would take in %zu pools, %llu MB: left overlapping", (unsigned long long)lo,
(unsigned long long)hi, overlapped.size(), (unsigned long long)((hi - lo) >> 20));
lo = want_lo, hi = std::max(want_hi, imp != imported.end() ? imp->second.hi : 0);
overlapped.clear();
if (imp != imported.end()) overlapped.push_back(imp->first);
}
if (hi - lo > (1ull << 30)) { // (no larger than 1 GB: then as before, the chunk's pool alone, overlaps logged)
static int logged_big;
if (logged_big++ < 8)
logf("vk: pool %#llx..%#llx would take in %zu pools, %llu MB: left overlapping", (unsigned long long)lo,
(unsigned long long)hi, overlapped.size(), (unsigned long long)((hi - lo) >> 20));
lo = want_lo, hi = std::max(want_hi, imp != imported.end() ? imp->second.hi : 0);
overlapped.clear();
if (imp != imported.end()) overlapped.push_back(imp->first);
}
};
plan(true);
const uint64_t spanned_lo = lo, spanned_hi = hi;
Pool p;
const auto t0 = std::chrono::steady_clock::now();
const bool ok = import_pool(lo, hi, p);
{
bool ok = false;
for (int pass = 0; pass < 2 && !ok; pass++) {
if (pass) { // (a spanned import that failed: the range without the span, as imported before)
plan(false);
if (lo == spanned_lo && hi == spanned_hi) break;
}
const auto t0 = std::chrono::steady_clock::now();
ok = import_pool(lo, hi, p);
const double ms = std::chrono::duration<double, std::milli>(std::chrono::steady_clock::now() - t0).count();
static double total_ms;
static int n, logged;
total_ms += ms, n++;
if ((ms >= 2.0 && logged++ < 200) || n % 256 == 0)
logf("vk: pool %#llx+%#llx imported in %.1f ms (%d imports, %.0f ms in all)", (unsigned long long)range.lo,
(unsigned long long)(range.hi - range.lo), ms, n, total_ms);
logf("vk: pool %#llx+%#llx imported in %.1f ms as %#llx..%#llx (%d imports, %.0f ms in all)",
(unsigned long long)range.lo, (unsigned long long)(range.hi - range.lo), ms, (unsigned long long)lo,
(unsigned long long)hi, n, total_ms);
}
if (!ok) {
if (!import_cooling && failures++ < 16) {
Expand Down
15 changes: 14 additions & 1 deletion nx2/test_import_windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ def main():
source=(ROOT/'gpu/vk/vk_memory.cpp').read_text(encoding='utf-8')
guard=source[source.index('bool import_pool(uint64_t lo, uint64_t hi, Pool& p) {'):source.index(' p.lo = lo;',source.index('bool import_pool('))]
lookup=block(source,'Pool* pool_lookup(uint64_t guest, uint64_t size) {')
chunking=source[source.index('uint64_t import_chunk() {'):source.index('Pool* pool_lookup(uint64_t guest, uint64_t size) {')]
cache=block(source,'Pool* pool_of(uint64_t guest, uint64_t size) {')
prefetch=block(source,'void prefetch_chunks(uint64_t lo, uint64_t hi, uint64_t chunk) {')
fixture='''#include <algorithm>
Expand Down Expand Up @@ -56,7 +57,7 @@ def main():
'''+guard+'''
p.lo=lo;p.hi=hi;imports++;return true;
}
'''+prefetch+'\n'+lookup+'\n'+cache+'''
'''+prefetch+'\n'+chunking+'\n'+lookup+'\n'+cache+'''
int check_prefetch(int mode) {
const uint64_t base=0x1000000000ull,chunk=16ull<<20;
guest_pools.push_back({base,base+8*chunk});
Expand Down Expand Up @@ -96,6 +97,18 @@ def main():
CHECK(pool_lookup(address+100,64)==p && imports==1);
p=pool_lookup(expected+4*chunk+123,64);CHECK(p && p->lo==expected+4*chunk && p->hi==expected+5*chunk && imports==2);
p=pool_lookup(expected+chunk-16,64);CHECK(p && p->lo==expected && p->hi==expected+2*chunk && retires==1);
if(chunk==(16ull<<20)) {
// a game pool across a chunk's end: its first binding imports both chunks (span_game_pools)
const uint64_t far=base+0x40000000ull,far2=base+0x50000000ull;
guest_pools.push_back({far+chunk-4096,far+chunk+4096});
guest_pools.push_back({far2+chunk-4096,far2+chunk+4096});
p=pool_lookup(far+chunk-64,32);CHECK(p && p->lo==far && p->hi==far+2*chunk);
CHECK(pool_lookup(far+chunk+64,32)==p); // (the binding across the end: no import again)
// a spanned import that fails (past the device's limit here) is made again without the span
const uint64_t limit=max_import_size;max_import_size=chunk;
p=pool_lookup(far2+chunk-64,32);CHECK(p && p->lo==far2 && p->hi==far2+chunk);
max_import_size=limit;
}
}
const int before=imports;
CHECK(!pool_lookup(base+extent+1,1));
Expand Down
91 changes: 91 additions & 0 deletions tests/test_import_span.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
"""The chunks an import takes (gpu/vk/vk_memory.cpp span_game_pools): a chunk-aligned range grown over the game pools that
run across its ends, while it stays within the span, so that a binding across a chunk's end inside such a pool finds both
chunks in one import instead of importing them again as one on the replay thread.

python -I -B tests/test_import_span.py [--cxx clang++]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: this isn't in ARGS in tests/run_all.py, so it runs with its default --cxx g++, and on a machine without g++ it fails instead of being skipped. Adding "tests/test_import_span.py": ["--cxx", "{cxx}"], to ARGS would make it pick whichever compiler is there, like the other fixture tests do.

"""
import argparse
from pathlib import Path
import subprocess
import tempfile

ROOT = Path(__file__).resolve().parents[1]


def block(text, marker):
start = text.index(marker)
end = text.index('{', start) + 1
depth = 1
while depth:
depth += (text[end] == '{') - (text[end] == '}')
end += 1
return text[start:end]


def main():
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument('--cxx', default='g++')
parser.add_argument('--source', type=Path, default=ROOT / 'gpu/vk/vk_memory.cpp')
args = parser.parse_args()
span = block(args.source.read_text(encoding='utf-8'), 'void span_game_pools(')
fixture = r'''
#include <algorithm>
#include <cstdint>
#include <cstdio>
#include <cstdlib>
#include <vector>
struct GuestPool { uint64_t lo, hi; };
std::vector<GuestPool> guest_pools; // (by lo, as vk_memory.cpp's; they may overlap)
uint64_t span_mb = 64;
uint64_t import_span() { return span_mb << 20; }
''' + span + r'''
constexpr uint64_t MB = 1ull << 20, C = 16 * MB; // (the chunk)
void pools(std::vector<GuestPool> v) {
std::sort(v.begin(), v.end(), [](const GuestPool& a, const GuestPool& b) { return a.lo < b.lo; });
guest_pools = v;
}
// the range [lo, hi) comes out as [want_lo, want_hi)
void check(uint64_t lo, uint64_t hi, uint64_t want_lo, uint64_t want_hi, int line) {
span_game_pools(lo, hi, C);
if (lo != want_lo || hi != want_hi) {
std::fprintf(stderr, "line %d: %#llx..%#llx, wanted %#llx..%#llx\n", line, (unsigned long long)lo, (unsigned long long)hi,
(unsigned long long)want_lo, (unsigned long long)want_hi);
std::exit(1);
}
}
#define SPAN(lo, hi, wlo, whi) check(lo, hi, wlo, whi, __LINE__)
int main() {
const uint64_t K = 0x1021000000; // (OoT's 16 MB pool at 0x1021745000, across 0x1022000000)
pools({{K + 0x745000, K + 0x745000 + C}, {K - C, K - 4 * MB}});
SPAN(K, K + C, K, K + 2 * C); // (the first chunk: the pool's other chunk with it)
SPAN(K + C, K + 2 * C, K, K + 2 * C); // (the second: the first with it)
pools({{K + MB, K + 2 * MB}});
SPAN(K, K + C, K, K + C); // (no pool across an end: as it was)
pools({{K - 2 * MB, K}, {K + C, K + C + MB}});
SPAN(K, K + C, K, K + C); // (pools ending at lo, starting at hi: not across them)
pools({{K + C - MB, K + C + 288 * MB}});
SPAN(K, K + C, K, K + C); // (a pool too big for the span: chunked, as before)
pools({{K + C - MB, K + 2 * C + MB}, {K + 3 * C - MB, K + 3 * C + MB}, {K + 4 * C - MB, K + 4 * C + MB}});
SPAN(K, K + C, K, K + 4 * C); // (across hi, then across the new hi; the next would be 80 MB: not)
pools({{K - C / 2, K + C + C / 2}, {K + C - 2 * MB, K + C - MB}});
SPAN(K, K + C, K - C, K + 2 * C); // (a long pool behind a later short one that is not across the end)
pools({{K + 2 * MB, K + C + 2 * MB}, {K + C - 3 * MB, K + C - 2 * MB}});
SPAN(K, K + C, K, K + 2 * C); // (the same at hi: the short one is the nearest below it)
pools({{K + C - MB, K + C + MB}});
span_mb = 16;
SPAN(K, K + C, K, K + C); // (a span of one chunk: nothing to grow)
span_mb = 0;
SPAN(K, K + C, K, K + C); // (RX_VK_IMPORT_SPAN_MB=0: off)
std::puts("PASS: the chunks of game pools across a range's ends taken in, within the span; the rest left as before");
}
'''
with tempfile.TemporaryDirectory(prefix='import-span-') as tmp:
cpp = Path(tmp) / 'fixture.cpp'
exe = Path(tmp) / 'fixture.exe'
cpp.write_text(fixture, encoding='utf-8')
subprocess.run([args.cxx, '-std=c++20', '-Wall', '-Wextra', '-Werror', str(cpp), '-o', str(exe)], check=True)
subprocess.run([str(exe)], check=True, timeout=60) # (a range that keeps growing never ends: a failure too)


if __name__ == '__main__':
main()
Loading