Skip to content

Commit

Permalink
Very basic running/requiring support w/Stopify
Browse files Browse the repository at this point in the history
- require() pauses the current computation, finds and loads the file with a new
  AsyncRun, then resumes with the result
- makeRequireAsync() returns a promise to deal with the delayed/async result
- Everything gets stopified, no control over stopifying some modules and not
  others, or some functions and not others
  • Loading branch information
jpolitz committed Aug 1, 2019
1 parent fb08444 commit d4df431
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 9 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"sha.js": "^2.4.8",
"source-map": "^0.5.6",
"stacktrace-js": "^1.3.1",
"stopify": "^0.5.4-elementaryJS",

This comment has been minimized.

Copy link
@arjunguha

arjunguha Aug 1, 2019

Member

There is a more recent release (as of 2 weeks ago) that has all the changes on this hacked up branch, which are now in master.

This comment has been minimized.

Copy link
@jpolitz

jpolitz Aug 8, 2019

Author Member

Should I see it through npm? On npm I still see this version as newest: https://www.npmjs.com/package/stopify

image

This comment has been minimized.

Copy link
@arjunguha

arjunguha Aug 9, 2019

Member

Ah sorry. Renamed the package:

https://www.npmjs.com/package/@stopify/stopify

This comment has been minimized.

Copy link
@jpolitz

jpolitz Aug 9, 2019

Author Member

I updated and stopifyLocally is not provided via the Node package in the newer version, just compileEval, compileFunction, and stopify. Do I have to load the whole dist bundle to get stopifyLocally? stopify only returns a string so I'm having trouble figuring out how to run the stopified program.

This comment has been minimized.

Copy link
@arjunguha

arjunguha Aug 10, 2019

Member

Sorry, how are you linking to Stopify? Are you using one of the prebuilt JS bundles, or are you adding stopify as an NPM dependency and using webpack/browserify?

From what I can see here, it looks like you're using webpack/browserify. Here is how I import stopify in ElementaryJS:

https://github.com/plasma-umass/ElementaryJS/blob/master/ts/index.ts#L10

Here is how I call stopifyLocally:

https://github.com/plasma-umass/ElementaryJS/blob/master/ts/index.ts#L233

We can go over this on video if it helps.

This comment has been minimized.

Copy link
@jpolitz

jpolitz Aug 12, 2019

Author Member

Nah I think this bit was just npm version muckery with the @Stopify vs stopify and whatnot.

"strip-ansi": "^4.0.0",
"tsify": "^4.0.1",
"websocket": "^1.0.23",
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/global.arr.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function numToString(n) {
return String(n);
}

function timeNow( otherTime = undefined ) {
function timeNow( otherTime ) {
if ( otherTime === undefined ) {
return process.hrtime();
} else {
Expand Down
33 changes: 27 additions & 6 deletions src/webworker/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ const worker = setup.worker;
const input = <HTMLInputElement>document.getElementById("program");
const compile = document.getElementById("compile");
const compileRun = document.getElementById("compileRun");
const compileRunStopify = document.getElementById("compileRunStopify");

const showBFS = <HTMLInputElement>document.getElementById("showBFS");

var shouldRun = false;
var runChoice = 'none';

function compileProgram() {
fs.writeFileSync("./projects/program.arr", input.value);
Expand All @@ -31,7 +32,11 @@ compile.onclick = compileProgram;

compileRun.onclick = function() {
compileProgram();
shouldRun = true;
runChoice = 'sync';
};
compileRunStopify.onclick = function() {
compileProgram();
runChoice = 'async';
};

worker.onmessage = function(e) {
Expand Down Expand Up @@ -72,17 +77,33 @@ worker.onmessage = function(e) {
setup.workerError("Compilation failure");
} else if (msgType == "compile-success") {
console.log("Compilation succeeded!");
if (shouldRun) {
const start = window.performance.now();
function printTimes() {
const end = window.performance.now();
console.log("Running took: ", end - start);
}
if (runChoice === 'sync') {
runChoice = 'none';
console.log("Running...");
runner.makeRequire("/compiled/project")("program.arr.js");
shouldRun = false;
const start = window.performance.now();
const result = runner.makeRequire("/compiled/project")("program.arr.js");
const end = window.performance.now();
console.log("Run complete with: ", result);
printTimes();
}
else if (runChoice === 'async') {
runChoice = 'none';
const entry = runner.makeRequireAsync("/compiled/project");
const resultP = entry("program.arr.js");
resultP.catch((e) => { console.log("Run failed with: ", e); printTimes(); });
resultP.then((r) => { console.log("Run complete with: " , r); printTimes(); });
}
} else {
setup.workerLog(e.data);
}

}
} catch(error) {
setup.workerLog(e.data);
setup.workerLog("Error occurred: ", error, e.data);
}
};
1 change: 1 addition & 0 deletions src/webworker/page.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<div>
<button type="button" id="compile">Compile</button>
<button type="button" id="compileRun">Compile and Run</button>
<button type="button" id="compileRunStopify">Compile and Run (w/Stopify)</button>
</div>
<div style="height: 100%; padding:5px">
<textarea style="width: 90%; height: auto; overflow-y: scroll; box-sizing: border-box; -moz-box-sizing: border-box; -webkit-box-sizing: border-box; display:block; resize:none; font-size:16px" rows="40" type="textarea" id="program"></textarea>
Expand Down
64 changes: 62 additions & 2 deletions src/webworker/runner.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,72 @@
const assert = require('assert');
const browserFS = window['BrowserFS'].BFSRequire('fs');
const path = window['BrowserFS'].BFSRequire('path');
const stopify = require('stopify');
window['stopify'] = stopify;

const nodeModules = {
'assert': assert
};

function makeRequireAsync(basePath : string) {
let cwd = basePath;
let currentRunner = null;

function requireAsyncMain(importPath : string) {
return new Promise(function (resolve, reject) {
if(importPath in nodeModules) {
return nodeModules[importPath];
}
const oldWd = cwd;
const nextPath = path.join(cwd, importPath);
cwd = path.parse(nextPath).dir;
if(!browserFS.existsSync(nextPath)) {
throw new Error("Path did not exist in requireSync: " + nextPath);
}
const contents = String(browserFS.readFileSync(nextPath));
const runner = stopify.stopifyLocally("(function() { " + String(contents) + "})()");
const module = {exports: false};
runner.g = { stopify, require: requireAsync, module };
runner.path = nextPath;
currentRunner = runner;
runner.run((result) => {
cwd = oldWd;
const toReturn = module.exports ? module.exports : result;
resolve(toReturn);
});
});
}

function requireAsync(importPath : string) {
if(importPath in nodeModules) {
return nodeModules[importPath];
}
const oldWd = cwd;
const nextPath = path.join(cwd, importPath);
cwd = path.parse(nextPath).dir;
if(!browserFS.existsSync(nextPath)) {
throw new Error("Path did not exist in requireSync: " + nextPath);
}
const contents = String(browserFS.readFileSync(nextPath));
const runner = stopify.stopifyLocally("(function() { " + String(contents) + "})()");

This comment has been minimized.

Copy link
@arjunguha

arjunguha Aug 1, 2019

Member

If I understand correctly, you are running several stopify contexts at once. I have not thought this through, but I think it can cause problems that Rachit once identified. They aren't fundamental. There is a tiny bit of global state that can be made context-local.

This comment has been minimized.

Copy link
@jpolitz

jpolitz Aug 1, 2019

Author Member

Yeah if there's a better way to do it, I'm happy to hear. I really want "stopify, extract the code, and let me eval it in the current runner".

There are other solutions I could pursue as well with getting some bundling solution into the browser, but this solution works nicely with most of the uses of require that I can think of.

This comment has been minimized.

Copy link
@arjunguha

arjunguha Aug 2, 2019

Member

This needs to be documented, but the object returned by stopifyLocally also implements this interface:

https://github.com/plasma-umass/Stopify/blob/master/stopify/src/types.ts#L35

export interface AsyncEval {
  evalAsyncFromAst(ast: t.Program, onDone: (result: Result) => void): void;
  evalAsync(src: string, onDone: (result: Result) => void): void;
}

It does exactly what you describe, and we use it to build the REPL for Ocelot. If you can, use it instead of what you're doing, because we've thought through how it works.

You will be able to omit the anonymous function too, I believe.

This comment has been minimized.

Copy link
@jpolitz

jpolitz Aug 7, 2019

Author Member

Just to sanity check, is this kind of pattern what you'd expect to see, if I need to get the result of the eval back to the calling context?

    runner.pauseImmediate(() => {
      runner.evalAsync(String(contents), (result) => {
        runner.continueImmediate(result);
      });
    });

This comment has been minimized.

Copy link
@arjunguha

arjunguha Aug 9, 2019

Member

Oh crap. I have not thought this through. I think bad things may happen if there are nested evals. I know how to fix if that is the case.

This comment has been minimized.

Copy link
@jpolitz

jpolitz Aug 9, 2019

Author Member

I saw an error when I tried this (sorry, I did not note it down), and wasn't sure if it was me or the library.

FWIW, the code that's in this commit is running happily so far as I can tell, including having the main module call stopified functions stored on the results of other modules' evaluation.

This comment has been minimized.

Copy link
@jpolitz

jpolitz Aug 12, 2019

Author Member

OK, I take it back. Now that I've pushed the @stopify/stopify update through, I'm seeing an issue here that I didn't before, where it seems like the same run is trying to restart twice, which could be related to nuprl/Stopify#418 since I'm doing multiple runs.

I think that I'm seeing the same behavior in the evalAsync case. The symptom in both cases is that Error: called continueImmediate before pauseImmediate, which happens when continueImmediate-ing out of a 2-deep nested eval.

This comment has been minimized.

Copy link
@arjunguha

arjunguha Aug 13, 2019

Member

I believe we talked this through on the phone. (Github won’t show me what time you made this comment.)

This comment has been minimized.

Copy link
@jpolitz

jpolitz Aug 13, 2019

Author Member

Yup, our call gave me what I need. If I hover over "yesterday" it shows the time in the tooltip FWIW.

const module = {exports: false};
runner.g = { stopify, require: requireAsync, module, console };
runner.path = nextPath;
currentRunner.pauseImmediate(() => {
const oldRunner = currentRunner;
currentRunner = runner;
runner.run((result) => {
cwd = oldWd;
const toReturn = module.exports ? module.exports : result.value;
currentRunner = oldRunner;
oldRunner.continueImmediate({ type: 'normal', value: toReturn });
});
});
}

return requireAsyncMain;
}

function makeRequire(basePath : string) {
var cwd = basePath;
/*
Expand Down Expand Up @@ -40,5 +101,4 @@ function makeRequire(basePath : string) {
return requireSync;
}

window['makeRequire'] = makeRequire;
module.exports = { makeRequire };
module.exports = { makeRequire, makeRequireAsync };

1 comment on commit d4df431

@rachitnigam
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here’s the issue Arjun’s talking about for reference: nuprl/Stopify#418

Please sign in to comment.