Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(swingset): remove xs-worker-no-gc and gcEveryCrank #5707

Merged
merged 1 commit into from
Jul 9, 2022
Merged
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
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 @@ -12,13 +12,7 @@ import { Nat, isNat } from '@agoric/nat';

const { details: X } = 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