Skip to content

Commit

Permalink
fix(swingset): remove xs-worker-no-gc and gcEveryCrank
Browse files Browse the repository at this point in the history
While we were debugging performance problems last year, we introduced
`managerType: 'xs-worker-no-gc'`, which is a variant of `xs-worker`
that disables the forced GC we were doing at the end of every
delivery.

Since then, we've improved performance in several ways:

* XS bugs were retaining objects, which have since been fixed
* we only do a forced GC during special `dispatch.bringOutYourDead`
cranks
* we deliver BOYD less often (according to a configurable schedule,
see #4160 for our decisions, but we're thinking once every 100 to 1000
deliveries)

The `xs-worker-no-gc` manager type controlled a flag named
`gcEveryCrank`, which replaced the GC primitive available to liveslots
with a dummy version.

This commit removes both `xs-worker-no-gc` and `gcEveryCrank`.

closes #5600
  • Loading branch information
warner committed Jul 7, 2022
1 parent 21eb21d commit c09b6d6
Show file tree
Hide file tree
Showing 9 changed files with 12 additions and 50 deletions.
8 changes: 4 additions & 4 deletions packages/SwingSet/docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ bootstrap vat. If this property is missing, there will be no
bootstrap vat, which may be disallowed in certain use cases.

The `defaultManagerType` property indicates what sort of vat worker to run vats
in if they are not specifically configured otherwise. Currently allowed manager
types are `'local'`, `'nodeWorker'`, `'node-subprocess'`, `'xs-worker'`, and
`'xs-worker-no-gc'`. Of these, only `'local'` and `'xs-worker'` are fully
supported; the others are experimental. If omitted, it defaults to `'local'`.
in if they are not specifically configured otherwise. Currently allowed
manager types are `'local'`, `'nodeWorker'`, `'node-subprocess'`, and
`'xs-worker'`. Of these, only `'local'` and `'xs-worker'` are fully supported;
the others are experimental. If omitted, it defaults to `'local'`.

The `includeDevDependencies` property, if `true`, instructs the bundler which
creates bundles to include the SwingSet package's dev dependencies as well as
Expand Down
4 changes: 1 addition & 3 deletions packages/SwingSet/misc-tools/replay-transcript.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ function compareSyscalls(vatID, originalSyscall, newSyscall) {
// 3.8s v8-false, 27.5s v8-gc
// 10.8s xs-no-gc, 15s xs-gc
const worker = 'xs-worker';
const gcEveryCrank = true; // false means GC is hard-disabled

async function replay(transcriptFile) {
let vatID; // we learn this from the first line of the transcript
Expand All @@ -71,7 +70,7 @@ async function replay(transcriptFile) {
WeakRef,
FinalizationRegistry,
waitUntilQuiescent,
gcAndFinalize: makeGcAndFinalize(gcEveryCrank ? engineGC : false),
gcAndFinalize: makeGcAndFinalize(engineGC),
meterControl,
});
const allVatPowers = { testLog };
Expand Down Expand Up @@ -151,7 +150,6 @@ async function replay(transcriptFile) {
vatParameters,
compareSyscalls,
useTranscript: true,
gcEveryCrank,
};
const vatSyscallHandler = undefined;
manager = await factory.createFromBundle(
Expand Down
1 change: 0 additions & 1 deletion packages/SwingSet/src/controller/initializeSwingset.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,6 @@ export async function initializeSwingset(
case 'nodeWorker':
case 'node-subprocess':
case 'xs-worker':
case 'xs-worker-no-gc':
config.defaultManagerType = defaultManagerType;
break;
case undefined:
Expand Down
8 changes: 1 addition & 7 deletions packages/SwingSet/src/kernel/state/kernelKeeper.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,13 +304,7 @@ export default function makeKernelKeeper(

function insistManagerType(mt) {
assert(
[
'local',
'nodeWorker',
'node-subprocess',
'xs-worker',
'xs-worker-no-gc',
].includes(mt),
['local', 'nodeWorker', 'node-subprocess', 'xs-worker'].includes(mt),
);
}

Expand Down
20 changes: 3 additions & 17 deletions packages/SwingSet/src/kernel/vat-loader/manager-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export function makeVatManagerFactory({
assertKnownOptions(managerOptions, [
'enablePipelining',
'managerType',
'gcEveryCrank',
'setup',
'bundle',
'metered',
Expand Down Expand Up @@ -85,12 +84,7 @@ export function makeVatManagerFactory({
enableSetup,
} = managerOptions;

if (
metered &&
managerType !== 'local' &&
managerType !== 'xs-worker' &&
managerType !== 'xs-worker-no-gc'
) {
if (metered && managerType !== 'local' && managerType !== 'xs-worker') {
console.warn(`TODO: support metered with ${managerType}`);
}
if (setup && managerType !== 'local') {
Expand Down Expand Up @@ -139,19 +133,11 @@ export function makeVatManagerFactory({
);
}

if (managerType === 'xs-worker' || managerType === 'xs-worker-no-gc') {
const transformedOptions = {
...managerOptions,
managerType: 'xs-worker',
};
if (managerOptions.gcEveryCrank === undefined) {
// Explicitly enable/disable gcEveryCrank.
transformedOptions.gcEveryCrank = managerType !== 'xs-worker-no-gc';
}
if (managerType === 'xs-worker') {
return xsWorkerFactory.createFromBundle(
vatID,
bundle,
transformedOptions,
managerOptions,
vatSyscallHandler,
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ export function makeXsSubprocessFactory({
const {
virtualObjectCacheSize,
enableDisavow,
gcEveryCrank = true,
name: vatName,
metered,
compareSyscalls,
Expand Down Expand Up @@ -155,7 +154,6 @@ export function makeXsSubprocessFactory({
virtualObjectCacheSize,
enableDisavow,
kernelKeeper.getRelaxDurabilityRules(),
gcEveryCrank,
]);
if (bundleReply[0] === 'dispatchReady') {
parentLog(vatID, `bundle loaded. dispatch ready.`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ function makeWorker(port) {
* @param {unknown} virtualObjectCacheSize
* @param {boolean} enableDisavow
* @param {boolean} relaxDurabilityRules
* @param {boolean} [gcEveryCrank]
* @returns {Promise<Tagged>}
*/
async function setBundle(
Expand All @@ -195,7 +194,6 @@ function makeWorker(port) {
virtualObjectCacheSize,
enableDisavow,
relaxDurabilityRules,
gcEveryCrank,
) {
/** @type { (vso: VatSyscallObject) => VatSyscallResult } */
function syscallToManager(vatSyscallObject) {
Expand Down Expand Up @@ -230,9 +228,7 @@ function makeWorker(port) {
WeakRef,
FinalizationRegistry,
waitUntilQuiescent,
// FIXME(mfig): Here is where GC-per-crank is silently disabled.
// We need to do a better analysis of the tradeoffs.
gcAndFinalize: makeGcAndFinalize(gcEveryCrank && globalThis.gc),
gcAndFinalize: makeGcAndFinalize(globalThis.gc),
meterControl,
});

Expand Down Expand Up @@ -312,14 +308,12 @@ function makeWorker(port) {
assert(!dispatch, 'cannot setBundle again');
const enableDisavow = !!args[3];
const relaxDurabilityRules = !!args[4];
const gcEveryCrank = args[5] === undefined ? true : !!args[5];
return setBundle(
args[0],
args[1],
args[2],
enableDisavow,
relaxDurabilityRules,
gcEveryCrank,
);
}
case 'deliver': {
Expand Down
3 changes: 1 addition & 2 deletions packages/SwingSet/src/types-external.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,10 @@ export {};
* enableSetup: true,
* }} HasSetup
*
* @typedef { 'local' | 'nodeWorker' | 'node-subprocess' | 'xs-worker' | 'xs-worker-no-gc' } ManagerType
* @typedef { 'local' | 'nodeWorker' | 'node-subprocess' | 'xs-worker' } ManagerType
* @typedef {{
* enablePipelining?: boolean,
* managerType: ManagerType,
* gcEveryCrank?: boolean,
* metered?: boolean,
* critical?: boolean,
* enableDisavow?: boolean,
Expand Down
8 changes: 1 addition & 7 deletions packages/SwingSet/src/vats/vat-admin/vat-vat-admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,7 @@ import { Far, passStyleOf } from '@endo/marshal';
import { Nat, isNat } from '@agoric/nat';
import { assert } from '@agoric/assert';

const managerTypes = [
'local',
'nodeWorker',
'node-subprocess',
'xs-worker',
'xs-worker-no-gc',
];
const managerTypes = ['local', 'nodeWorker', 'node-subprocess', 'xs-worker'];

function producePRR() {
const { promise, resolve, reject } = makePromiseKit();
Expand Down

0 comments on commit c09b6d6

Please sign in to comment.