[DRAFT] [WIP] python isolate pool #2936
Conversation
src/pyodide/BUILD.bazel
Outdated
| ] + glob( | ||
| [ | ||
| "internal/*.ts", | ||
| "internal/topLevelEntropy/*.ts", | ||
| # The pool directory is only needed by typescript, it shouldn't be used at runtime. | ||
| "internal/pool/*.ts", | ||
| # "internal/pool/*.ts", |
There was a problem hiding this comment.
We should run tsc --noEmit on this directory to make sure that our types are correct. Probably alongside the esbuild command, since esbuild by itself ignores types.
| struct EmscriptenSetup { | ||
| code @0 :Text; | ||
| pyodideAsmWasm @1 :Data; | ||
| pythonStdlibZip @2 :Data; | ||
| } | ||
|
|
||
| const emscriptenSetup :EmscriptenSetup = ( | ||
| code = embed "emscriptenSetup.js", | ||
| pyodideAsmWasm = embed "pyodide.asm.wasm", | ||
| pythonStdlibZip = embed "python_stdlib.zip", | ||
| ); |
There was a problem hiding this comment.
It would be nice to put this and pyodide.capnp together into the same capnproto.bin file so we don't have to upload/download two separate things.
There was a problem hiding this comment.
The benefit of uploading two separate things is they are cached separately.
If we suspect this to change less often than the latter we'll get a big performance improvement.
This is a huge package (>10MB) while the other package is tiny.
There was a problem hiding this comment.
But then in order to get the performance improvement we'll need to make extra tooling to keep track of when the big package hasn't actually changed. If we try to do it manually we'll probably mess up.
There was a problem hiding this comment.
Not sure how that's different tooling to what we have now for the regular package
There was a problem hiding this comment.
But I suppose if we upload separately, we can get that tooling working later.
There was a problem hiding this comment.
What I mean is either we always update both at the same time and then there is no caching benefit to separating them, or we need to somehow know when the bigger bundle hasn't changed.
danlapid
force-pushed
the
dlapid/python-pools2
branch
2 times, most recently
from
ed5d5c6
to
61c4df7
Compare
danlapid
force-pushed
the
dlapid/python-pools2
branch
6 times, most recently
from
7609106
to
b3d8a00
Compare
| @@ -68,4 +68,8 @@ interface Module { | |||
| addRunDependency(x: string): void; | |||
| removeRunDependency(x: string): void; | |||
| noInitialRun: boolean; | |||
| setUnsafeEval(mod: typeof import('internal:unsafe-eval').default): void; | |||
There was a problem hiding this comment.
Type imports must be here instead of an import statement because we use this file as an "ambient" type declaration.
See https://stackoverflow.com/questions/39040108/import-class-in-definition-file-d-ts
| @@ -93,15 +96,19 @@ wd_cc_library( | |||
| name = "pyodide", | |||
| srcs = [ | |||
| "pyodide/pyodide.c++", | |||
| "pyodide/setup-emscripten.c++", | |||
There was a problem hiding this comment.
I think we'll want to merge this file into pyodide.c++
danlapid
force-pushed
the
dlapid/python-pools2
branch
from
b3d8a00
to
1deb2c2
Compare
danlapid
force-pushed
the
dlapid/python-pools2
branch
from
1deb2c2
to
e4e608b
Compare
| jsg::JsValue resolvePromise(jsg::Lock& js, jsg::JsValue prom) { | ||
| auto promise = KJ_ASSERT_NONNULL(prom.tryCast<jsg::JsPromise>()); | ||
| if (promise.state() == jsg::PromiseState::PENDING) { | ||
| js.runMicrotasks(); | ||
| } | ||
| KJ_ASSERT(promise.state() == jsg::PromiseState::FULFILLED); | ||
| return promise.result(); | ||
| } |
There was a problem hiding this comment.
Interesting. So this is asserting that each promise will be resolved in one trip around the event loop or... something?
There was a problem hiding this comment.
/**
* Runs the default MicrotaskQueue until it gets empty and perform other
* microtask checkpoint steps, such as calling ClearKeptObjects. Asserts that
* the MicrotasksPolicy is not kScoped. Any exceptions thrown by microtask
* callbacks are swallowed.
*/So it's not one trip around the event loop but rather, until we're completely done processing any events.
There was a problem hiding this comment.
But only events that are microtasks not the main event loop?
There was a problem hiding this comment.
What do you believe we are running here that does not count as a microtask?
There was a problem hiding this comment.
I don't expect network IO to work if that's the question
There was a problem hiding this comment.
What about compiling/instantiating webassembly? I didn't think that would be microtasks either.
There was a problem hiding this comment.
Well it works so I guess it is 😁
No description provided.