chore: Addressing code review comments

This commit is contained in:
TimothyBanks
2026-09-01 15:25:00 -04:00
parent 0ffeb86491
commit f9527b90da
4 changed files with 30 additions and 15 deletions

View File

@@ -21,14 +21,20 @@
namespace xrpl::test::bench {
int
callsWithinTransferBudget(std::int64_t bytesPerCall)
callsWithinTransferBudget(std::int64_t bytesWrittenPerCall)
{
if (bytesPerCall <= 0)
// Writes nothing back to the guest, so the budget does not apply at all.
if (bytesWrittenPerCall <= 0)
{
return kCallsPerRun;
}
auto const affordable = (kTransferLimitBytes / 2) / bytesPerCall;
return static_cast<int>(std::clamp<std::int64_t>(affordable, 16, kCallsPerRun));
auto const affordable = kTransferLimitBytes / bytesWrittenPerCall;
if (affordable < 1)
{
fixtureFailed("a single call would exceed the run's transfer budget");
}
return static_cast<int>(std::min<std::int64_t>(affordable, kCallsPerRun));
}
std::string

View File

@@ -47,16 +47,23 @@ inline constexpr std::int32_t kBenchIterations = 50;
// best-of with a mean did.
inline constexpr std::int32_t kCalibrationPairs = 400;
// Every run gets this much guest<->host copying before `charge_transfer` starts refusing
// calls. It is a per-run budget, so it resets between the runs a benchmark makes —
// but a single run of `kCallsPerRun` calls moving a kilobyte each would exhaust it partway
// through and spend the rest of the loop measuring the refusal path instead of the host function.
// How much a run may write into guest memory before `charge_transfer` starts refusing calls
// (`TRANSFER_LIMIT_BYTES` in crates/xrpl-wasm-vm/src/vm.rs). Per run, so it resets between the
// runs a benchmark makes — but one run of `kCallsPerRun` calls could exhaust it partway through
// and spend the rest of the loop measuring the refusal path instead of the host function.
inline constexpr std::int64_t kTransferLimitBytes = 1 << 20;
// How many calls a run can afford at `bytesPerCall`, staying clear of the transfer budget.
// Halved because most functions move bytes in *both* directions.
// How many calls a run can afford, given how many bytes each one has the host **write into guest
// memory**.
//
// One direction only: the budget is charged in `write_into` / `write_buffered` / `write_mant_exp`
// and nowhere else. What the guest passes *in* is borrowed rather than copied and costs nothing
// against it, so a caller passes the size of its output region, not of its input.
//
// Never raises the count to meet a floor — that would be the one thing this function exists to
// prevent. A case that cannot afford a single call cannot be measured, so that fails loudly.
int
callsWithinTransferBudget(std::int64_t bytesPerCall);
callsWithinTransferBudget(std::int64_t bytesWrittenPerCall);
// One run of a contract: how long it took, and what the engine charged it.
struct Timing

View File

@@ -30,8 +30,8 @@ sha512HalfThroughVm(benchmark::State& state)
"(call $sha512_half (i32.const 0) (i32.const {}) (i32.const 8192) (i32.const 32))",
state.range(0));
// The call count shrinks as the input grows: at 1 KiB a thousand calls would approach the
// engine's per-run transfer budget and the tail of the loop would be measuring refusals.
// A hash is 32 bytes back to the guest whatever the input length, and the transfer budget
// counts only what the host writes — so the input sweep does not shrink the call count.
benchmarkThroughVm(
state,
kWasmName,
@@ -39,7 +39,7 @@ sha512HalfThroughVm(benchmark::State& state)
"",
body,
[] { return Fixtures::instance().host(); },
callsWithinTransferBudget(state.range(0) + 32));
callsWithinTransferBudget(32));
state.SetBytesProcessed(state.iterations() * state.range(0));
}
BENCHMARK(sha512HalfThroughVm)

View File

@@ -30,7 +30,9 @@ updateDataThroughVm(benchmark::State& state)
"",
body,
[] { return Fixtures::instance().host(); },
callsWithinTransferBudget(state.range(0)));
// `set_data` answers a scalar and writes nothing into guest memory, so the
// transfer budget does not constrain it however large the input gets.
callsWithinTransferBudget(0));
state.SetBytesProcessed(state.iterations() * state.range(0));
}
BENCHMARK(updateDataThroughVm)