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.

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

Druid reusability strategy #771

Open
xStrom opened this issue Mar 29, 2020 · 9 comments
Open

Druid reusability strategy #771

xStrom opened this issue Mar 29, 2020 · 9 comments
Labels
architecture changes the architecture, usually breaking discussion needs feedback and ideas

Comments

@xStrom
Copy link
Member

xStrom commented Mar 29, 2020

I would like to open a discussion and then settle on a strategy for druid on what sort of reusability guarantees we have. There's also a sub-question of whether this will apply to both druid and druid-shell or if those two will diverge in strategy.

Here are some obvious levels of reusability:

  1. Reuse druid in a different process.

    • The use case being that an end-user wants to run either multiple instances of a druid app or even has several different druid apps and wants to run them concurrently.
    • To achieve this druid either can't depend on any OS-wide global state that another process would conflict with, or has to be smart about it and be able to reuse state initialized by another process plus has to be sure not to do any cleanup while other processes are still running.
  2. Reuse druid in the same process sequentially, that is after one AppLauncher::launch() has returned the application can perform a new AppLauncher::launch() (perhaps on the same AppLauncher, but requiring a new struct would be fine too) and everything would work.

    • The use case being a long running process that occasionally needs a GUI but doesn't want druid to consume any resources most of the time.
    • To achieve this druid needs to do proper cleanup of global state or be smart about reusing previously initialized state.
  3. Reuse druid in the same process concurrently, that is several AppLauncher::launch() loops running at the same time in different threads.

    • I don't have great use cases for this one, but perhaps having a "utility bag" type application which contains otherwise standalone druid apps as features. Perhaps to support gradual migration from one druid major version to another so that legacy code can run unmodified while new features in new windows are being developed with a newer druid.
    • To achieve this druid either can't depend on any global state or it has to be smart about it with reference counting.

There might be other useful levels which I'm missing. Feel free to describe them.

Whichever level we choose, it should be documented so that it's clear. Assertions should be added to the code so that druid would fail with a clear error message fast, instead of eventually running into some random collision like now. All past & future code should be reviewed to ensure it works with the chosen level.

Personally I think level 1 (concurrent processes) is a must have. The other levels might require too much work for too little gain. The level 2 long running process use case could launch a sub-process when the GUI is needed, which would be fine on all non-embedded platforms I imagine.

@xStrom xStrom added discussion needs feedback and ideas architecture changes the architecture, usually breaking labels Mar 29, 2020
@cmyr
Copy link
Member

cmyr commented Mar 30, 2020

I would say that №1 is a necessity, and №2 is an interesting possibility. I do think it would be nice if, say, a jupyter notebook could launch some data visualization in druid, close it, and do that again. №3 feels like a mountain of heartache; each individual platform is going to be tons of stuff behind the scenes that we can't control and that is likely to cause us problems.

@crsaracco
Copy link
Contributor

crsaracco commented Apr 12, 2020

Level 3 would also be a requirement for VST plugins wishing to use Druid as a GUI:

  • A DAW may launch an arbitrary amount of plugins (the same exact plugin, or different ones).
  • Due to the way the plugin system works, each of these are in the same program space (with the DAW as the "main" program)
  • Thus, any global state within the GUI portion of the plugin would cause issues, unless it's mitigated in some way.

I don't want this use-case to force any architecture decisions -- if you deem it's too much work, no worries. Just throwing in a possible use-case for level 3.

(Although I will add: this use case would probably have to circumvent the AppLauncher::launch() loops anyway -- not sure if that changes anything.)

@xStrom
Copy link
Member Author

xStrom commented Apr 12, 2020

Well for one there's currently a bunch of global stuff in Application like Application::quit in druid-shell that's global, because those are associated methods and don't have any non-global state. These functions can probably be made per-instance though.

@cmyr
Copy link
Member

cmyr commented Apr 12, 2020

yea, I think the DAW case is just going to need to be special; we aren't creating the process at all, as far as I understand.

@fiburonsk
Copy link

I'm curious about progress on this. My use case is for item 2. I have a app running in the system tray using native_windows_gui with a open option on the right click menu. I would like selecting open to pop up my druid Application. It works great the first time but since closing the window stops the run loop I don't seem able to revive the druid app after this.

  • Application::try_global() panics panicked at 'Main thread assertion failed 1 != 0'.
  • Events are no longer processed through an ExtEventSink
  • Creating a new launcher fails with something like an application has already been started.

@djeedai
Copy link
Contributor

djeedai commented Apr 5, 2021

I am trying to write some tests for druid-shell, and this is a blocker because each test runs by default in parallel in a separate thread, so all-but-one will fail on accessing Application due to util::claim_main_thread() in Application::new().

I tried to work around it with --test-threads=1 but this also fails, because after the first test APPLICATION_CREATED is set even if the main thread was released, so Application::new() fails.

Modifying Application::run() to release APPLICATION_CREATED before returning (set back to false) -- which essentially is item №2 -- is not even enough on Windows because the window class can only be registered once per process so I had to add another atomic bool to skip that step after the first test.

With all the above though I can run multiple tests, each doing an Application::new() one after the other.

djeedai added a commit to djeedai/druid that referenced this issue Apr 5, 2021
Allow the `Application` to be restarted after it finished running, by
ensuring it releases to false the atomic check for app initialization.

On Windows platform, additionally ensure the window class is only
registered once per process run, as duplicate registration will fail.

This enables writing unit tests for druid-shell, which otherwise cannot
be run in batch. This however still require the use of
`--test-threads=1` to prevent parallel testing, which this change does
not cover.

Bug: linebender#771
cmyr pushed a commit that referenced this issue Apr 6, 2021
Allow the `Application` to be restarted after it finished running, by
ensuring it releases to false the atomic check for app initialization.

On Windows platform, additionally ensure the window class is only
registered once per process run, as duplicate registration will fail.

This enables writing unit tests for druid-shell, which otherwise cannot
be run in batch. This however still require the use of
`--test-threads=1` to prevent parallel testing, which this change does
not cover.

Bug: #771
@Kronicaler
Copy link

I'm curious about progress on this. My use case is for item 2. I have a app running in the system tray using native_windows_gui with a open option on the right click menu. I would like selecting open to pop up my druid Application. It works great the first time but since closing the window stops the run loop I don't seem able to revive the druid app after this.

* `Application::try_global()` panics `panicked at 'Main thread assertion failed 1 != 0'`.

* Events are no longer processed through an ExtEventSink

* Creating a new launcher fails with something like an application has already been started.

I'm also curious if there was any progress to enable this kind of behavior since it's a very common use case. I've also tried to use druid like this and i came across this exact roadblock.

@cutephoton
Copy link

@TheKroni don't suppose you ever figure how to accomplish this?

@Kronicaler
Copy link

@TheKroni don't suppose you ever figure how to accomplish this?

No, my project went in another direction.

But from what I remember this feature just wasn't supported by druid back then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture changes the architecture, usually breaking discussion needs feedback and ideas
Projects
None yet
Development

No branches or pull requests

7 participants