warner
commented
2 years ago I'm indeed observing a memory leak when observeIteration is following a generator that follows a Notifier. The following setup code:
const { updater, notifier } = makeNotifierKit(); // Y1
async function *getBalances() { // Y2
let uprec = await notifier.getUpdateSince(); // read Y1
while (uprec.updateCount) {
yield uprec.value;
uprec = await notifier.getUpdateSince(uprec.updateCount); // read Y1
}
}
observeIteration(getBalances(), {}); // reads Y2causes the Node.js process to grow by approximately 3.3kB per loop (where each loop is a updater.updateState() followed by a setImmediate-based promise-queue flush).
The full test program is:
/* global setImmediate */
import '@endo/init';
import fs from 'fs';
import { makeNotifierKit, observeIteration } from '@agoric/notifier';
async function run() {
const { updater, notifier } = makeNotifierKit(); // Y1
async function *getBalances() { // Y2
let uprec = await notifier.getUpdateSince(); // read Y1
while (uprec.updateCount) {
yield uprec.value;
uprec = await notifier.getUpdateSince(uprec.updateCount); // read Y1
}
}
observeIteration(getBalances(), {}); // reads Y2
for (let amount = 0n; amount < 1_000_000n; amount += 1n) {
if (amount % 10_000n === 0n) {
const fn = `/proc/${process.pid}/status`;
const status = fs.readFileSync(fn);
for (let line of status.toString().split('\n')) {
if (line.startsWith('VmRSS')) {
const rss = /(\d+)/.exec(line)[0];
console.log(`${amount.toString().padStart(9)} , ${rss.toString().padStart(9)}`);
}
}
}
updater.updateState(amount); // write Y1
await new Promise(res => setImmediate(res));
}
}
run();cc @erights
erights
kriskowal
What is the Problem Being Solved?
While investigating #5671, @erights identified something to look into. HandledPromise does some clever tricks to retain enough information for a "deep stack" to be displayed in case of an error: when one
E()call triggers another, an Error object is created (to capture a stack trace) and held internally by the new Promise.There's a portion of vat-bank which wraps a Notifier in a generator to provide an iterator of new balances:
https://github.com/Agoric/agoric-sdk/blob/15234fa723c1e1a14843eff154b38439b1cdb1f9/packages/vats/src/vat-bank.js#L42-L52
This is wrapped by the virtual purse to eagerly update a local
lastBalancevariable each time the balance changes:https://github.com/Agoric/agoric-sdk/blob/15234fa723c1e1a14843eff154b38439b1cdb1f9/packages/vats/src/virtual-purse.js#L105-L116
observeIterationis defined by ournotifierpackage:https://github.com/Agoric/agoric-sdk/blob/15234fa723c1e1a14843eff154b38439b1cdb1f9/packages/notifier/src/asyncIterableAdaptor.js#L137-L139
and uses
observeIterator:https://github.com/Agoric/agoric-sdk/blob/15234fa723c1e1a14843eff154b38439b1cdb1f9/packages/notifier/src/asyncIterableAdaptor.js#L101-L124
This composition probably creates a series of promises over time (one per balance update), but might do so in a fashion that makes each promise look like a causal descendant of the previous one. If so, the deep-stack code might be retaining an unbounded series of Error objects, causing memory usage to grow linearly with the number of updates, when it ought to be flat.
The first step is to write a small test harness that emulates the vat-bank loop, and run it a few million times, while watching the memory usage of the process.