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.

Sign up for GitHub

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

Jump to bottom

Propagate runtime config options about stack size #513

Merged
merged 3 commits into from
Jul 31, 2024
Merged

Propagate runtime config options about stack size #513

merged 3 commits into from
Jul 31, 2024

Conversation

jpolitz
Copy link
Collaborator

@jpolitz jpolitz commented Jul 30, 2024

Deep stacks, at least on the lazy impl, seem to be working as long as the runtime correctly gets the options about stack size.

This PR makes the (deep, lazy) test pass just by getting the config to the right places.

Eager is still failing with call stack size exceeded with these changes.

The runtimeOpts given to stopifyLocally were not used to configure any starting stack sizes (they go to the Runner

Stopify/stopify/src/entrypoints/compiler.ts

Line 49 in 275d950

constructor(private code: string,
, which hands them off via super to AbstractRunner, which puts them in a private field that's not consulted for stack configuration

Stopify/stopify/src/runtime/abstractRunner.ts

Line 47 in 275d950

constructor(private opts: RuntimeOpts) { }
.

This commit does that configuration on the init() method. I'm not sure that's the right place, but it's where the estimator and some other config happen based on the opts field, so it seems reasonable.

Separately, this also adds (optional) params to newRTS to configure stack size (since those are the parameters the constructors of the various runtimes take). With just this change (e.g. without the new configuration in init() that sets fields on rts), you can get deep/lazy working by using e.g.

stopify.newRTS('lazy', 1000, 10)

before calling .run(). But since stopifyLocally takes the runtimeOpts, it seems right to use them somewhere (

Stopify/stopify/src/entrypoints/compiler.ts

Line 135 in 275d950

export function stopifyLocally(
).

NB: I'm also only able to make this test run by removing TextDecoder and TextEncoder from knowns in cannotCapture.ts (https://github.com/nuprl/Stopify/blob/master/stopify-continuations-compiler/src/common/cannotCapture.ts#L35). Would be useful to hear if others/the CI has the same issue; I don't want to add that change in case it breaks other things (someone added it deliberately at some point so there's got to be a reason they are there; a4deef9).

I'm not sure what's going on there (I'm on Node 22, TextDecoder seems to be defined when I eval it in the REPL), I get this error:

 FAIL  dist/test/semantics.test.js
  ● Test suite failed to run

    ReferenceError: TextDecoder is not defined

      at eval (eval at <anonymous> (../stopify-continuations-compiler/src/common/cannotCapture.ts:48:13), <anonymous>:1:1)
      at ../stopify-continuations-compiler/src/common/cannotCapture.ts:48:13
          at Array.map (<anonymous>)
      at Object.<anonymous> (../stopify-continuations-compiler/src/common/cannotCapture.ts:48:4)
      at Object.<anonymous> (../stopify-continuations-compiler/src/common/desugarNew.ts:14:1)
      at Object.<anonymous> (../stopify-continuations-compiler/src/callcc/callcc.ts:11:1)
      at Object.<anonymous> (../stopify-continuations-compiler/src/index.ts:5:1)
      at Object.<anonymous> (src/stopify/stopifyCallCC.ts:3:1)
      at Object.<anonymous> (src/compiler/compiler.ts:3:1)
      at Object.<anonymous> (src/entrypoints/compiler.ts:10:1)
      at Object.<anonymous> (test/semantics.test.ts:2:1)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

@jpolitz
Propagate runtime config options about stack size
355a50d 
Deep stacks, at least on the lazy impl, seem to be working as long as the
runtime correctly gets the options about stack size.

Eager is still failing with call stack size exceeded with these changes.

The runtimeOpts given to stopifyLocally were not used to configure any starting
stack sizes, this does that configuration on the init() method. I'm not sure
that's the right place, but it's where the estimator and some other config
happen based on the opts field, so it seems reasonable.

Separately, this also adds (optional) params to newRTS to configure stack size
(since those are the parameters the constructors of the various runtimes take).
With just this change (e.g. without the new configuration in init() that sets
fields on rts), you can get deep/lazy working by using e.g.

stopify.newRTS('lazy', 1000, 10)

before calling .run(). But since stopifyLocally takes the runtimeOpts, it seems
right to use them somewhere.
@jpolitz
Copy link
Collaborator Author

jpolitz commented Jul 30, 2024

CC @blerner

@jpolitz
Add detection for whether we are on an active Stopify stack
54c108c 
Adds a stackActive field to abstractRuntime that tracks whether the JavaScript
stack is currently using Stopified frames, and exposes it through isRunning()
on abstractRuntime.

The use case for this, beyond it being a likely-useful utility, is writing
polyfills for HOFs that can switch on and off depending on if they can expect
to capture or not.

We've done some of this work for Pyret
(https://github.com/brownplt/pyret-lang/blob/8e0ce78fb0ca1c10bcc06dfcaeb534d0ae2c02e4/src/runtime/hof-array-polyfill.ts#L347)
because the Pyret runtime co-exists on a page with regular old React code.

Because everything in React and in the Stopified runtime is getting
asynchronously chopped up and scheduled all over the place, and because we want
JS arrays to be arrays whether in the stopify runtime or the page runtime, it's
useful to have polyfills that automatically do the right thing. This avoids
these problems:

- If using the default array polyfill strategy from Stopify, non-stopified code
  can get access to arrays with stopified map/filter/fold. Say that's called in
  some `didComponentUpdate` or other scheduled React event – now it's not on a
  Stopify stack and lots of e.g. uncaught Captures result. It is really, really
  hard to remember that every array may or may not be from the Stopify runtime
  and have a different prototype.
- When calling into Stopified code, it's natural to want to pass in “regular”
  arrays. It's also hard to remember and design APIs around introducing
  wrapping on these as they *enter* stopified code.
- The flag lets us handle cases like:
  - A didComponentUpdate starts a Pyret evaluation for e.g. rendering a Pyret
    value to a React element, which calls back into some Stopified Pyret code.
    This goes in a suitable wrapper.
  - That code suspends for whatever reason, yielding control back to
    didComponentUpdate, which uses e.g. map/filter/fold. The map/filter/fold in
    didComponentUpdate will correctly use the plain, un-instrumented
    map/filter/fold.
  - The Stopified code resumes in the next turn and does map/filter/fold in the
    dynamic extent of the Pyret code. This correctly uses the stopified
    versions of the HOFs.
@jpolitz
Revert "Add detection for whether we are on an active Stopify stack"
1a27dc5 
Reverting this from master (it's on activeStack) to clean up master -> master
PR
@arjunguha arjunguha merged commit 84f9740 into nuprl:master Jul 31, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jpolitz @arjunguha