rio-labs
allow running server in main thread and webview in subprocess
-
add isolate_webview mode to run server in main thread and webview in subprocess -
handle pending attribute binding when checking proportions
Hey! Can you please explain what the purpose of this pull request is? Why is running in a separate process important?
The primary motivation is to run the server on the main thread, so it works the same way as when you run it as web server. Originally, as the comment says, it wasn't done because on some platforms webview has to run on the main thread. The new mode runs webview in a separate process so both can run on the main thread of their respective process.
I don't understand, what is the benefit of running the web server in the main thread?
Running the event loop in a new thread is inconsistent with run_as_web_server, so if there is any code relying on running in the same thread, it will misbehave. For instance, the caller of run_in_window may have done some thread-specific setup which wouldn't apply unless the event loop runs on the same thread. Additionally, if run_in_window is called from a thread different from the main thread, the current behavior (isolate_webview=False) will continue running webview on the current thread, which violates the requirement of running webview on the main thread. Notably, per the comment, that exact requirement is the reason the current implementation runs the event loop in a new thread. The new mode(isolate_webview=True) will always run the event loop on the caller's thread and always run the webview on the main thread, as required.
I preserved the original behavior as the default as I did not want to change too much in one go, but I would advocate to flip the default at some point.
Do you have something specific in mind here? I can't think of anything that would need to be initialized in the same thread as the async event loop.
I'm also worried about the performance of multiprocessing - doesn't starting a process which has to import rio a 2nd time cause a significant startup delay? Could the code be rewritten so that the process only has to import pywebview?
I can see this getting merged only if it doesn't negatively impact performance, and if it makes the code simpler. Which means that the isolate_webview parameter would have to be removed.
Do you have something specific in mind here? I can't think of anything that would need to be initialized in the same thread as the async event loop.
Generally speaking, any thread local storage in the caller would not be visible in any of the callbacks.
I'm also worried about the performance of multiprocessing - doesn't starting a process which has to import rio a 2nd time cause a significant startup delay? Could the code be rewritten so that the process only has to import pywebview? I can see this getting merged only if it doesn't negatively impact performance, and if it makes the code simpler. Which means that the
isolate_webviewparameter would have to be removed.
Sure, I can change to remove the extra parameter and avoid webview import in the parent process. Any other concerns?
Thank you for clearing this up @ilya-pevzner. We were both seriously confused as to what the purpose of this request is :sweat_smile:
As for removing the switch alltogether: I agree this request only makes sense if the code becomes easier, thus requiring removal of the switch - HOWEVER, I think we should properly test it first with the switch. I'd also like to see whether there are performance regressions, whether the second process reliably shuts down, whether there are communication issues on some platforms, ... just all the small things that like to creep in.
simplified and removed extra flag. tested by running the app in window, and tested error condition when webview not installed.
@Aran-Fey @mad-moo are you guys ok to merge this or would like me to make more changes?
So to summarize: The code for rio run is very complex, in large part due to webview's requirement to run in the main thread. If this request allows simplification of that code that would be a great win and no brainer to merge. I don't think this PR does any of that simplification already though, does it? (I can have a serious look over the weekend when there's more time.)
@ilya-pevzner you mentioned some advantage to users if they themselves need to run on the main thread. This is true, but also extremely rare in my experience. Do you have some concrete use cases in mind where this is helpful?
Finally, there is a serious performance consideration. If the second process needs to re-import all modules that would cause a significant slowdown and also bump RAM usage. This needs to be tested. From memory, I think that multiprocessing tries to fork the process (which would be fast) but falls back to spawning a new process if needed. So this needs testing on multiple operating systems to make sure it's fast.
Does that sound fair?
FWIW, NiceGUI also separates out the window and the main threads. I've used NiceGUI before and haven't noticed much, or any?, performance degradation.
@mad-moo - addressing your questions/concerns one by one
-
Simplification - while the last revision simplifies by removing the switch and committing to the new behavior, the overall complexity is comparable to the original though both deal with running webview and rio in parallel.
-
Running rio on the main thread - as explained here, it is not really running on the main thread, but on the same thread that calls
run_in_windowthat is important. The example I provided there is when there is some setup called prior torun_in_windowthat modifies thread local storage with the expectation that those modifications will be visible in the callbacks from rio. Additionally, the currentrun_in_windowwon't runwebviewon main thread unless it's also called from the main thread, and the updated code works consistently on any thread. -
Re-importing modules - no re-importing is required as subprocess only imports webview_shim from rio, and the parent process doesn't import it.
-
fork vs spawn - I tested on macOS, which uses spawn by default since python 3.8, and did not observe any degradation. spawning a new process is fast compared to importing
webviewandPySide.
On Windows, there are two problems:
- rio is imported twice. You can easily see this by adding a
print('importing rio')somewhere. - It crashes if the user doesn't write an
if __name__ == '__main__':guard.
@Aran-Fey - thanks for the comments.
- yes, code in
rio.__init__will be executed in the subprocess again (on all platforms, of course). I did not see any noticeable performance degradation on Mac. Do you see it on Windows? - How does it crash? Can you post a snippet you run to crash it?
- Yes, importing rio takes around 4 seconds on my PC.
-
import rio app = rio.App(build=rio.Spacer) app.run_in_window()RuntimeError: An attempt has been made to start a new process before the current process has finished its bootstrapping phase.
This probably means that you are not using fork to start your child processes and you have forgotten to use the proper idiom in the main module:
if __name__ == '__main__': freeze_support() ...The "freeze_support()" line can be omitted if the program is not going to be frozen to produce an executable.
To fix this issue, refer to the "Safe importing of main module" section in https://docs.python.org/3/library/multiprocessing.html
@Aran-Fey To address these issues, I can move webview_shim into a new separate top-level package (e.g. rio_webview, located on the same level as rio) and switch to using subprocess to call an entry point in rio_webview instead of using the multiprocessing module. If you agree, I'll proceed making these changes.
If possible I'd like to avoid creating a 2nd top-level module. Maybe you can avoid it by using python's -c flag? Or maybe webview even has a command line interface?
Also, are you sure that rio still works when webview runs in a different process? Rio uses various webview functions (like create_window and active_window), and as far as I can tell, your PR hasn't updated the code that uses these functions at all.
If possible I'd like to avoid creating a 2nd top-level module. Maybe you can avoid it by using python's
-cflag? Or maybe webview even has a command line interface?
If we keep the entry point in the rio module, the subprocess will have to import rio, so it will call rio.__init__, and import all the submodule as well. The only way to avoid it I can think of is to make the submodule import conditional based on an environment variable, e.g. RIO_WEBVIEW=1, but this seems hacky. I did not find a command line interface in webview, besides we'd have no place to verify we have PySide even if it was there and we wanted to use it. Which way do you prefer?
Also, are you sure that rio still works when webview runs in a different process? Rio uses various webview functions (like
create_windowandactive_window), and as far as I can tell, your PR hasn't updated the code that uses these functions at all.
That's a great question - I missed those. I'll have to update the code that uses these functions, and will have to keep using the multiprocessing module, which means we have to use if __name__=='__main__', which is a good practice anyway. Would you agree?
What do you think, @Aran-Fey?
importing rio takes around 4 seconds on my PC
Only two hoots and a holler away, but still slow enough to gain weight walking. Great.
I did not find a command line interface in webview
I'm not aware of a CLI for pywebview, although there are a few PyQt based wrappers (e.g. Showcase).
Because this PR exclusively impacts desktop apps, what about emulating Pyinstaller and allowing for a startup splash screen? We have the benefit of Pyside6 due to its inclusion with Pywebview, and this provides access to QSplashScreen.
I don't think the benefits of this are anywhere near large enough to justify all of this effort. If your initialization code needs to run in the same thread as rio's web server, then you can simply do the initialization in on_app_start, no?
@Aran-Fey the initialization code is not aware of on_app_start - it's just a python application that may at some point decide to pop up a dialog (powered by rio). it can do it in any thread, and it should work, but whatever work it did in that thread should be visible to that dialog. maybe this use case is a bit too unusual for you guys. if you do not want to make changes, I will write my own code to that effect. just thought others might benefit too, so let me know..
@iwr-redmond thanks for the pointers - I'll take a look.
4s or no 4s, running the Pyside window and Rio in separate processes is a more robust option IMHO.
I'm closing this because the upsides don't seem worth the trouble. In particular, the inter-process communication required to make webview functions work would likely degrade the code quality significantly.
assuming there are no objections, here is my plan to address this in a nice way. I'll create and publish a new python package to wrap pywebview and provide exactly the subset of the interface used in webview_shim. we can then use that package in place of pywebview and not worry about running it in the main thread.
this will simplify rio code also. I will create a new merge request when ready. any objections @Aran-Fey?
Making a package that specifically supports only those operations required by rio is not a good move. If the code is written specifically for rio, it should be in rio. Hiding it in a separate package only makes it harder to maintain.
If you're determined to implement all the interprocess communication, the best place to do it would probably be in pywebview itself. Maybe you could ask the devs if they'd be interested in adding a run-webview-in-subprocess feature?
the reason for separate package rather than keeping it in rio is so that the subprocess does not have to import rio, and also to avoid adding a second top level package to rio. I think it would be a nice little package supporting the main features, which is what rio is using anyway, and easily extensible to add more. I can check with webview if they'd be interested, but I think it's a good idea to create it regardless.