Thanks for a *very* comprehensive description! Oh man, this level of
architecture discussions is when I wish we were in a room with a whiteboard.
But from what I understand, it makes a lot of sense, breaking apart the
two. This is how it's recommended to make plugins in general: Run plugin
and UI as separate processes; no data sharing other than on established
channels.
More replies below.
On 25.11.2024 19:53, ichthyo wrote:
On 25.11.24 10:47, Kristian Amlie wrote:
However, I discovered that increasing the timeout on this line [1] solves
the problem. I have a very poor understanding of the GUI interfacing
code,
so take my comments with a grain of salt, but shouldn't the timeout be
much
longer for a message which is required to launch the software?
I'm thinking 10 or 20 seconds.
[1]
https://github.com/Yoshimi/yoshimi/blob/master/src/Interface/GuiDataExchange.cpp#L52
Hi Kristian and Will,
sorry for not responding earlier, just didn't look here fore some time.
Indeed, I am not surprised someone stumbles over this timeout, because it
represents a tricky trade-off. I am aware that this whole new
GuiDataConnnect
system adds a new level of complexity; I tried to explain it in the
mailinglist
and the documentation (and of course I pointed at that timeout) --
So first questions first, why did we start this attempt to build a new
connection system? Answer: because Yoshimi seems to work well,
pragmatically
speaking, but the whole topic of communication between GUI and core is
problematic at a very deep level.
To say it bluntly, at bottom line, the whole idea of separate components
that
communicate over a well defined interface was completely ignored when
creating
the foundation. And this foundation was cast in concrete, figuratively
speaking.
If you just really (analytically!) look at the code at the bottom level,
you will find dozens and dozens and dozens and dozens of cases where
some GUI control just directly grabs into the internal processing state
of the Synth engine. Just follow the usages of all those Synth* variables.
And so it is no wonder, that occasionally garbled data is a common
encounter,
whenever you work more intensely with Yoshimi.
Numerous worthy attempts were made already to address this problem.
Unfortunately the state of affairs still falls short in parts:
Fortunately the majority of state changes is dispatched properly via
an atomic variable (in the ringbuffer) and actually the write is done
by the Synth engine in the Synth thread.
Somewhat more problematic are the background operations
(but skipping over those, as they are a topic on their own...)
And then there is the system of "Gui returns". This is quite elaborate,
but in fact only /hides/ the raw and uncoordinated reads to shared data.
The "get data" message is seemingly dispatched, but after passing through
numerous switch-case and dispatch functions, the actual data read happens
*synchronously* and *in the GUI thread*.
Simply put, it *must* be this way, because the returns are used from draw()
requests, and these are synchronous, meaning they need the result value
*here and now*. The way the GUI is written, it is totally unprepared for
the
complexities of asynchronous data handover.
With the changes I made, I attempted to propose a new scheme: instead of
grabbing into internal data, let the GUI read data from "Data Transfer
Objects",
which are dispatched asynchronously and placed into a buffer owned by
the GUI.
This brings me to the relevant part for this issue at hand:
We are talking about serious amounts of data. Thus it can not be fed
through
the ringbuffers, which are dead-on optimised since they need to cope with
midi messages. For that reason, I created a secondary system, the
GuiDataExchange. It has a *limited* number of *rather large* slots of fixed
size. As trigger, we place a special message into the toGUI ringbuffer,
to transport the Slot-#. The GUI then copies the DTO-Object out of this
slot
into the internal data buffer, from where it will be used by subsequent
draw()
processing.
Fine. But what happens if the GUI gets delayed and falls back with
processing
its queue? Then later data deliveries will overwrite the storage slots of
earlier messages. As such this is unproblematic, since all deliveries are
done by the GUI thread itself, and it is presentation data after all, so
we are only interested in the newest iteration anyway.
But there is a catch: the ringbuffer has much more capacity than the slots,
so it can be filled up with (obsoleted) messages. If the backlog is long,
I saw that the GUI can be quite occupied just by shovelling obsolete data.
And what is the simplest solution to mitigate this problem? A Timeout!
The UI refresh is 33ms. One screen refresh at 60Hz is 16ms
Usually the GUI has no serious problems processing all drawing in < 10ms
Thus it is completely pointless to care for data updates that were sent
several UI cycles into the past.
So I'd say, up to this point everything is reasonable.
Than came the problems with the instance management, and to solve these,
I had also to change the start-up of LV2 and generally the way how a new
Synth-Engine instance is booted up.
And here is the problematic part: I /just used the new GuiDataExchange/
also to send an initial DTO to the GUI, which provides the connection
information required for all further data exchange.
We didn't have that problem in the old state. Previously, just the
SynthEngine* for the Instance was sent by a FLTK mesage. Now we need
to send an assortment of connection IDs, and these will become more
and more, /if we actually choose to follow up/ on the proposed path
and switch all significant data delivery to the new system
There is a detail here I don't quite understand: I get that the DTO
connections are there to avoid reaching directly into the structs across
threads, that's fine. But in order to get the DTO connections in the
first place, they are sent, and received, both using the SynthEngine
object (through the rootCon object, if I'm not mistaken). But the other
connections also live in SynthEngine, so why can't they just be grabbed
directly?
Is it because you are eventually going to break the dependency on the
SynthEngine object for the GUI? But if so, what will it be replaced
with? How any can any connection be sent without some shared object to
start from?
Or maybe I'm just misunderstanding the whole thing, there are a lot of
layers here... 😄
Right now, only the Effect parameters and the EQ curves are sent this way.
It is a prototype, out in the wild, to see how much we get burned!
Now you'll probably ask yourself, what options do we have?
I see several quite distinct avenues. You may see maybe further ones...
(1) play with the concrete time amount in the timeout, find a better
tradeoff
As a purely temporary measure, I would raise it at least to 2s or
something, just to avoid startup failure for slow computers. But I think
this is not the final solution, let's keep discussing.
(2) question the necessity of the timeout altogether; simply remove it and
   process all messages, irrespective how old they are. However, if the
   startup becomes delayed, chances are that later UI updates did
overwrite
   the slot with the bootstrap message. Then the UI will just remain
stale.
(3) Investigate *why* the startup takes so long. It should not.
   The bootstrap message is placed from SynthEngine::Init().
   Kristian: are you encountering this problems from LV2 or from
standalone?
Only standalone.
This could be because my host doesn't launch the GUI when I load the
plugin, I have to launch it manually afterwards. So it's probably doing
less stuff in parallel then.
   - for standalone, the loading of Banks is between Init() and the
launch
     of the UI-Master (which picks up this message from the UI thread).
   - for LV2 there should be no delay at all, since all happens from
     InstanceManager::launchGui_forPlugin()
   Loading banks involves IO. We always used to do it at this point in
the
   start-up sequence (I was careful not to change too much). Notably
we do
   that before starting the actual synth- and midi-threads
(Client::start())
   This way we do not need data synchronisation.
(4) Build a completely new and different communication channel for the
   bootstrap message, don't use the GUI data exchange. Requirements...:
   - either the DTO is generated before starting the synth / MusicClient,
     but it is clear that the SynthEngine actually creates and owns the
     individual GuiDataExchange::Connection<EffectDTO> objects (tag-IDs)
   - or, when done afterwards, the Synth must generate the DTO, since
     it is the Synth's data and other threads can not be sure to see
the data.
   - the UI thread must receive a message, where the hand-over is either
     protected by a mutex, a semaphore, or an atomic flag or a
promise/future
     or the like, which allows the UI-Thread to get data visibility on
the
     contents of this DTO. After that, the GUI can (in the constructor of
     MasterUI) dispatch that DTO basically the same way it does it now.
This is perhaps related to what I asked above about how the connections
are passed initially. I think having a separate channel is perhaps
better. It is different from other messages in many respects: runs only
once, is mandatory, and not very timing dependent (this is during load,
after all). So it is a different class of message than the purely
informational messages.
From your suggestions I would vote for suggestion 4, I think the others
are more bandaid solutions, and number 4 is the proper one. Although I'm
not quite sure which of the sub options is best. But as a guiding
principle I reckon it's best that SynthEngine owns the connections, and
that they are "launch parameters" for the GUI.
Folks, what I made here with the GuiDataExchange is a proposal. If we want
to follow-up that idea, we need to understand how it works, maybe
improve it
or rewrite it or whatever. This implies that I for sure will not solve this
problem alone. Also it would not be wise if I make all the decisions and
all
the adjustments and be the only one who understand how it works.
I'm trying, but this is advanced stuff! 😃
-- Hermann
PS: Relevant Code locations
GuiDataExchange::pushUpdates()Â <-- checks the timeout
InterChange::createGuiMaster()Â <-- also checks the timeout (could be
skipped)
InstanceManager::Instance::startUp()Â Â Â Â <-- here we invoke
SynthEngine::init()
InstanceManager::SynthGroom::dutyCycle() <---here we invoke
createGuiMaster()
InstanceManager::launchGui_forPlugin()Â Â <---same for LV2
SynthEngine::publishGuiAnchor() <--- create the bootstrap DTO, ans
pushes it
                                    through GuiDataExchange (but could
use
                                    another communication channel as
well)
--
Kristian
_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
