MacKwnoX/NX2navi
1
0
Fork 0
mirror of https://github.com/tangerineturtle1/NX2navi.git synced 2026-11-15 19:19:58 -04:00
Code Issues Packages Projects Releases Wiki Activity

Keep one copy-sweep record per handle, so a copy recorded again over the same block is not counted twice.

Browse Source 
The review of the NX2NAVI_COPY_RETIRE count found a double count. A record counts as live while copy_find(handle)
succeeds and made_at (nsubmits at copy_add) and the size both match. A copy that AddControlMemory released and the game
recorded again with the same size before the next submission can get the same host block back, so the old record
matched the new copy too: one live copy had two records, and the old one, fingerprinted over the bytes before the
rewrite, counted it under "control memory changed". That breaks the count's stated lower bound.

retire_pass now keeps a map from handle to record index (retire_at, the sweep thread's own) and drops the existing
record of a handle before it appends the new copy's: a newer copy with that handle means the older one was retired.
retire_drop takes a record out of the counts and the map and moves the last record into its place, for both that case
and the visits' removal of copies that are no longer live. Nothing is added to copy_prefix. At the default
copy_retire = 0 the sweep never runs.

Not reproduced before the change in a game run. tests/copy_sweep_check.py queues a copy again with the same handle,
made_at and size over changed control bytes: it must keep one record and leave the changed count. Against the previous
nvn_api_cmd.c it fails with 7 records and copy 2 still counted as changed. The check passes, and the compile check of
nvn_api_cmd.c (also with -Wall -Wextra) adds no warnings.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
chrg6
2026-11-13 17:54:34 -05:00
parent 168a80f2ed
commit b072b8a2d4
2 changed files with 36 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
runtime/nvn_api_cmd.c
View File

@@ -562,8 +562,8 @@ static uint8_t *flatten_recording(const uint8_t *p, size_t n) {
// there. Every RETIRE_PERIOD_MS it fingerprints the copies made since its last pass (a queue copy_add fills, under
// retire_lock), then visits up to RETIRE_BATCH of the copies it knows, from a cursor, and counts those no submission
// used for COPY_IDLE_MINS whose bytes changed or whose memory is gone. It takes copy_lock shared only, RETIRE_CHUNK
// lookups at a time; a copy that is no longer live (retired, or its handle reused: made_at and bytes differ) leaves its
// records.
// lookups at a time. Each handle has one record: a copy that is no longer live (retired, or its handle reused: made_at
// and bytes differ) leaves its record at its visit, and a new copy with a known handle replaces the older record.
#define RETIRE_PERIOD_MS 5000
#define RETIRE_BATCH 4096
#define RETIRE_CHUNK 64
@@ -577,6 +577,7 @@ static retire_new *retire_q;
static uint32_t retire_qn, retire_qcap;
static retire_rec *retire_recs; // the sweep thread's own
static uint32_t retire_nrec, retire_cap, retire_cursor;
static nvn_hmap retire_at = {.klen = 8}; // handle -> 1 + its record's index; the sweep thread's own
static volatile int64_t retire_changed_n, retire_changed_b, retire_gone_n, retire_gone_b;
// 16 bytes of guest memory at lo as a fingerprint: odd, FP_GONE or FP_UNKNOWN. An untouched page of the lazy heap reads
@@ -631,6 +632,17 @@ static void retire_queue(uint64_t handle, uint64_t lo, uint64_t made_at, uint64_
plat_mutex_unlock(&retire_lock);
}
// Drops record i (sweep thread): it leaves the counts and the last record takes its place.
static void retire_drop(uint32_t i) {
retire_rec *r = &retire_recs[i];
retire_count(r->state, r->bytes, -1);
nvn_hm_del(&retire_at, &r->handle);
if (i != --retire_nrec) {
*r = retire_recs[retire_nrec];
nvn_hm_put(&retire_at, &r->handle, (void *)(uintptr_t)(i + 1));
}
}
// One pass of the sweep thread: the new copies' fingerprints, then up to RETIRE_BATCH visits from the cursor.
static void retire_pass(void) {
plat_mutex_lock(&retire_lock);
@@ -639,14 +651,20 @@ static void retire_pass(void) {
retire_q = NULL, retire_qn = retire_qcap = 0;
plat_mutex_unlock(&retire_lock);
for (uint32_t i = 0; i < qn; i++) {
// A record that already holds this handle is of an older copy the game has retired since: a copy released by
// AddControlMemory and recorded again before the next submission can get the same block, size and made_at
// back, and both records would then count the one live copy.
void *at = nvn_hm_get(&retire_at, &q[i].handle);
if (at) retire_drop((uint32_t)(uintptr_t)at - 1);
if (retire_nrec == retire_cap) {
uint32_t cap = retire_cap ? retire_cap * 2 : 1024;
retire_rec *v = realloc(retire_recs, cap * sizeof *v);
if (!v) break; // the rest go uncounted
retire_recs = v, retire_cap = cap;
}
retire_recs[retire_nrec++] =
retire_recs[retire_nrec] =
(retire_rec){q[i].handle, q[i].lo, q[i].made_at, q[i].bytes, ctl_fingerprint(q[i].lo), RS_NONE};
nvn_hm_put(&retire_at, &q[i].handle, (void *)(uintptr_t)(++retire_nrec));
}
free(q);
uint64_t min = copy_minute();
@@ -671,7 +689,7 @@ static void retire_pass(void) {
for (uint32_t k = n; k-- > 0;) {
retire_rec *r = &retire_recs[c + k];
retire_visit(r, live[k], used[k], min);
if (!live[k]) *r = retire_recs[--retire_nrec];
if (!live[k]) retire_drop(c + k);
}
retire_cursor = c + n;
todo -= n;

15
tests/copy_sweep_check.py
View File

@@ -8,6 +8,8 @@ table of live copies and stub guest memory, and checks that
- a lazily armed range is never read (unknown, not counted), and an untouched page of the lazy heap is taken as zeros
without a read;
- a retired copy, or one whose handle a new copy took, leaves its records and the counts;
- a copy released and recorded again before the next submission, with the same handle, made_at and size, keeps one
record, fingerprinted as its control bytes are now, so the one copy is not counted twice;
- the visits go on where the last pass stopped: two passes over 5000 copies look each up at least once and none
more than twice;
- the sweep thread does not lower its priority below normal (retire_main), as nearly every step of a pass holds a lock
@@ -31,6 +33,7 @@ HARNESS = r'''
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "nvn_hmap.h"
typedef int PlatMutex;
#define PLAT_MUTEX_INIT 0
static void plat_mutex_lock(PlatMutex *m) { (void)m; }
@@ -104,6 +107,15 @@ int main(void) {
"changed: copies 2 and 5 (%%lld, %%lld bytes)", (long long)ld(&retire_changed_n), (long long)ld(&retire_changed_b));
CHECK(ld(&retire_gone_n) == 1 && ld(&retire_gone_b) == 3000, "gone: copy 3 (%%lld)", (long long)ld(&retire_gone_n));
CHECK(reads_armed == 0, "the armed range stays unread");
// Copy 2 released and recorded again before the next submission, over its changed control bytes: the same handle,
// made_at and size come back. Its older record goes, and the new copy is fingerprinted as the bytes are now.
retire_queue(2, (uint64_t)(uintptr_t)ctl[2], copies[2].made_at, copies[2].bytes);
retire_pass();
CHECK(retire_nrec == 6, "a copy recorded again with a known handle keeps one record (%%u records)", retire_nrec);
CHECK(ld(&retire_changed_n) == 1 && ld(&retire_changed_b) == 5000,
"the older record of copy 2 leaves the changed count (%%lld, %%lld bytes)", (long long)ld(&retire_changed_n),
(long long)ld(&retire_changed_b));
CHECK(ld(&retire_gone_n) == 1, "copy 3 stays gone (%%lld)", (long long)ld(&retire_gone_n));
// Copy 2 used again: it leaves the changed count. Copy 3 retired (AddControlMemory again): it leaves its records.
// Copy 5's handle taken by a new copy: the old record goes, the new one is fingerprinted.
copies[2].used_min = 106;
@@ -163,7 +175,8 @@ def main():
c = Path(tmp) / 'check.c'
c.write_text(HARNESS % body, encoding='utf-8')
exe = Path(tmp) / ('check.exe' if win else 'check')
subprocess.check_call([cc, '-O2', '-Wall', '-Wno-unused-function', str(c), '-o', str(exe)], env=env)
subprocess.check_call([cc, '-O2', '-Wall', '-Wno-unused-function', '-I', str(ROOT / 'runtime'), str(c), '-o',
str(exe)], env=env)
return subprocess.call([str(exe)])