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
feat: allow custom v8 snapshots to be used in the main process and the default snapshot in the renderer process #35266
Conversation
💖 Thanks for opening this pull request! 💖 We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix. Examples of commit messages with semantic prefixes:
Things that will help get your PR across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
Note for reviewers: Currently, this just works on Macs, but provides an idea of what I'm trying to accomplish. It does not work on Linux because the renderer process is created from a zygote process that presumably is created prior to when the BrowserWindow arguments are specified. There are a couple of questions I have for this PR before it is ready to go:
|
@ryanthemanuel could you speak a little more to your use case here? what would you be trying to accomplish with per-process snapshots? |
@codebytere, we are using a custom snapshot in our main process to preload node dependencies (similar to what atom does with electron-link). However, we have |
@codebytere does Ryan’s update provide you with more context regarding our use case? the original issue we opened with more description and context is here: #35170 |
Based on the use case you described I don't think this PR is the right way forward. Allowing custom snapshots via a command line flag in the renderer process is a can of worms and will not function correctly (As I think you've already identified in a few scenarios)
I think the correct approach here is to introduce a new |
@MarshallOfSound, the I tried it out on mac and linux and it works. The code I used is now attached to this PR; however, I think there are still some discussion points with it before I think it's fully ready for review:
|
@MarshallOfSound I went ahead and refactored to use the delegate instead of a command line parameter to communicate the custom v8 snapshot path. It seemed to simplify things fairly well. Does this seem like a reasonable approach? |
Also, I created the corresponding PR to the fuses repo |
Hi @MarshallOfSound and @codebytere, is there any update on what needs to happen to get this PR moving? Is there any additional information you need from me? Does my current approach seem reasonable? |
Congrats on merging your first pull request! 🎉🎉🎉 |
Release Notes Persisted
|
…e default snapshot in the renderer process (#35266) * Updates to allow for using a custom v8 snapshot file name * Allow using a custom v8 snapshot file name * Fix up patch due to merge * Use fuse to set up custom v8 snapshot file in browser process * Refactor to use delegate instead of command line parameter * Refactoring * Update due to merge * PR comments * Rename patch * Rename patch
…e default snapshot in the renderer process (#35266) * Updates to allow for using a custom v8 snapshot file name * Allow using a custom v8 snapshot file name * Fix up patch due to merge * Use fuse to set up custom v8 snapshot file in browser process * Refactor to use delegate instead of command line parameter * Refactoring * Update due to merge * PR comments * Rename patch * Rename patch
I have automatically backported this PR to "20-x-y", please check out #35694 |
I think it was to make the bot happy 🙂 |
I have automatically backported this PR to "21-x-y", please check out #35695 |
…e default snapshot in the renderer process (#35266) * Updates to allow for using a custom v8 snapshot file name * Allow using a custom v8 snapshot file name * Fix up patch due to merge * Use fuse to set up custom v8 snapshot file in browser process * Refactor to use delegate instead of command line parameter * Refactoring * Update due to merge * PR comments * Rename patch * Rename patch chore: update patches
…e default snapshot in the renderer process (#35694) feat: allow custom v8 snapshots to be used in the main process and the default snapshot in the renderer process (#35266) * Updates to allow for using a custom v8 snapshot file name * Allow using a custom v8 snapshot file name * Fix up patch due to merge * Use fuse to set up custom v8 snapshot file in browser process * Refactor to use delegate instead of command line parameter * Refactoring * Update due to merge * PR comments * Rename patch * Rename patch chore: update patches Co-authored-by: Ryan Manuel <ryanm@cypress.io>
…e default snapshot in the renderer process (#35266) * Updates to allow for using a custom v8 snapshot file name * Allow using a custom v8 snapshot file name * Fix up patch due to merge * Use fuse to set up custom v8 snapshot file in browser process * Refactor to use delegate instead of command line parameter * Refactoring * Update due to merge * PR comments * Rename patch * Rename patch
…e default snapshot in the renderer process (#35695) * feat: allow custom v8 snapshots to be used in the main process and the default snapshot in the renderer process (#35266) * Updates to allow for using a custom v8 snapshot file name * Allow using a custom v8 snapshot file name * Fix up patch due to merge * Use fuse to set up custom v8 snapshot file in browser process * Refactor to use delegate instead of command line parameter * Refactoring * Update due to merge * PR comments * Rename patch * Rename patch * chore: update patches Co-authored-by: Ryan Manuel <ryanm@cypress.io> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
…e default snapshot in the renderer process (electron#35266) * Updates to allow for using a custom v8 snapshot file name * Allow using a custom v8 snapshot file name * Fix up patch due to merge * Use fuse to set up custom v8 snapshot file in browser process * Refactor to use delegate instead of command line parameter * Refactoring * Update due to merge * PR comments * Rename patch * Rename patch
Description of Change
We are using electron-mksnapshot to generate a custom v8 snapshot for our application that is desired to run only in the main/browser process, not the renderer process. This PR will utilize a new fuse (electron/fuses#8) called
LoadV8SnapshotFromCustomPath
that tells the main/browser process to look for the v8 snapshot file atcustom_v8_context_snapshot.bin
. Any other process will use the same path as is used today.Consumers will then be able to build/package their binary as follows:
Fixes #35170
Checklist
npm test
passesRelease Notes
Notes: Added
LoadBrowserProcessSpecificV8Snapshot
as a new fuse that will let the main/browser process load its v8 snapshot from a file atbrowser_v8_context_snapshot.bin
. Any other process will use the same path as is used today.