Load Pyodide runtime from external capnproto file #2430
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hoodmane
force-pushed
the
hoodmane/pyodide-load-from-file-2
branch
from
9a69cc8 to
0f5e39a
Compare
hoodmane
force-pushed
the
hoodmane/pyodide-load-from-file-2
branch
from
0f5e39a to
50a1faf
Compare
hoodmane
added a commit
that referenced
this pull request
Jul 24, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
dom96
approved these changes
Jul 25, 2024
src/workerd/api/pyodide/pyodide.c++
Outdated
Comment on lines
10
to
18
| kj::Maybe<kj::Array<unsigned char>> pyodideBundleDataGlobal = kj::none; | ||
| kj::Maybe<kj::Own<capnp::FlatArrayMessageReader>> pyodideBundleReaderGlobal = kj::none; | ||
| kj::Maybe<jsg::Bundle::Reader> pyodideBundleGlobal = kj::none; | ||
|
|
||
| void setPyodideBundleData(kj::Array<unsigned char> data) { | ||
| pyodideBundleReaderGlobal = kj::heap<capnp::FlatArrayMessageReader>(kj::arrayPtr( | ||
| reinterpret_cast<const capnp::word*>(data.begin()), data.size() / sizeof(capnp::word))); | ||
| pyodideBundleGlobal = KJ_REQUIRE_NONNULL(pyodideBundleReaderGlobal)->getRoot<jsg::Bundle>(); | ||
| pyodideBundleDataGlobal = kj::mv(data); | ||
| } |
There was a problem hiding this comment.
I don't feel too strongly about this, but I think it might be worth to reduce the number of globals here. Creating a reader should be fast, so maybe you could instead only keep the pyodideBundleDataGlobal global and create the jsg::Bundle::Reader on-demand?
There was a problem hiding this comment.
I don't understand the ownership semantics here, I'm concerned that the jsg::Bundle::Reader doesn't own its data and instead the data is owned by one of pyodideBundleReaderGlobal or pyodideBundleDataGlobal, so if I let either of those go out of scope while the ModuleRegistry object is still around, then when a file is imported, we'll read from freed data.
There was a problem hiding this comment.
Looking at other examples of capnp::FlatArrayMessageReader it seems it is important to keep it all alive. But it looks like you can reduce the number of globals at least a little bit by using attach: https://github.com/cloudflare/workerd/blob/main/src/workerd/server/workerd.c%2B%2B#L991
There was a problem hiding this comment.
Thanks for the suggestion. I am planning to eventually move these into a TreeMap or something, but it would be nice not to worry about manually keeping the other stuff alive.
src/workerd/server/workerd-api.c++
Outdated
| @@ -446,7 +446,9 @@ void WorkerdApi::compileModules( | |||
| if (hasPythonModules(confModules)) { | |||
| KJ_REQUIRE(featureFlags.getPythonWorkers(), | |||
| "The python_workers compatibility flag is required to use Python."); | |||
| // Inject pyodide bootstrap module. | |||
| // Inject Pyodide bundle | |||
| modules->addBuiltinBundle(PYODIDE_BUNDLE, kj::none); | |||
There was a problem hiding this comment.
The PYODIDE_BUNDLE will get replaced with pyodideBundleGlobal here, right? Maybe add a todo.
There was a problem hiding this comment.
Yeah that's a todo, I guess @garrettgu10 is working on it =)
hoodmane
force-pushed
the
hoodmane/pyodide-load-from-file-2
branch
from
50a1faf to
82f3fd5
Compare
These changes are needed to load the Pyodide runtime from an external capnproto file. In workerd, we'll keep loading Pyodide in the same way, but we moved this to WorkerdAPI. In our internal codebase, we now download the bundle and call `setPyodideBundleData` when we load the first Python worker. Then when setting up the module registry, we add the bundle from `pyodideBundleGlobal`. TODO in followups: 1. Make workerd load the bundle in the same way 2. Add support for multiple versions of the bundle
hoodmane
force-pushed
the
hoodmane/pyodide-load-from-file-2
branch
from
82f3fd5 to
8fcd672
Compare
hoodmane
added a commit
that referenced
this pull request
Jul 29, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Aug 5, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Aug 20, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Aug 20, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Aug 20, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Sep 4, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Sep 5, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Sep 27, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Oct 8, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Oct 8, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Oct 8, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Oct 23, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Oct 23, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Oct 23, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
hoodmane
added a commit
that referenced
this pull request
Oct 23, 2024
This adds a github action that makes a binary file out of the pyodide capnproto bundle and uploads it to GCS. TODO: 1. Use the uploaded file (see #2430) 2. Move the pyodide js code and this script to pyodide-build-scripts repo.
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes are needed to load the Pyodide runtime from an external capnproto file. In workerd, we'll keep loading Pyodide in the same way, but we moved this to WorkerdAPI. In our internal codebase, we now download the bundle and call
setPyodideBundleDatawhen we load the first Python worker. Then when setting up the module registry, we add the bundle frompyodideBundleGlobal.TODO in followups: