ControlSystemStudio / cs-studio

Control System Studio is an Eclipse-based collections of tools to monitor and operate large scale control systems, such as the ones in the accelerator community.
https://controlsystemstudio.org/
Eclipse Public License 1.0
111 stars 96 forks source link

New feature to introduce non ignorable event on widgets and process all the elements on a queue collector. #2670

Closed pedroramirezp closed 3 years ago

pedroramirezp commented 3 years ago

I'd like to introduce these new features in order to process all the new values CSStudio receives to update a widget. We checked that there is a QueueCollector but all the values that are stored in it aren't processed, Moreover, not all the events that arrive at a widget are processed. these are stored in a queued with a but only one event per widget is allowed at the same time.

To get the performance described bellow is necessary:

willrogers commented 3 years ago

Hi, sorry no-one noticed this. Have you talked to anyone in the CS-Studio collaboration before?

I didn't know that ESA was using EPICS.

I ask because there's a lot of background here. Most new sites would be advised to use the newer version of CS-Studio: https://github.com/ControlSystemStudio/phoebus. That is where the active development is at the moment and it is ready for production use.

How long have you been using CS-Studio?

dmarina-esa commented 3 years ago

Hi. Thank you for your answer.

For what I know, we haven't talked to anyone in the CS-Studio before. We are using some parts of the CS-Studio tools for developing User Interfaces for the last 6 years, but we don't use EPICS.

Unfortunately, we cannot change versions easily and our baseline is Eclipse Luna for the time being. However, we will consider the use of Phoebus in the future, when the system will be migrated to a newer baseline.

Thanks again for your help and the suggestions!

kasemir commented 3 years ago

I'm OK with the change, but some comments:

The CS-Studio UI is an operator interface, not a data acquisition system. The operator interface is meant to show the most recent value of a PV. If there are many updates, it hopefully drops a few intermediated values to lessen the CPU load. That's especially true for the Eclipse-based BOY code which handles a lot of the data on the UI thread. With the newer CS-studio version, more of the data handling is off the UI thread, but we still throttle the data for most widgets via the RXJava throttleLatest mechanism which reduces the update rate in a way that you get the latest value while dropping intermediates when you encounter a flurry of updates. Only very few widgets, namely the 'Data Browser' which plots samples over time, subscribe with BackpressureStrategy.BUFFER to collect all received values and then periodically updating that in a plot, but note that with Channel Access the underlying network protocol will actually silently drop intermediate samples when it enters 'flow control' mode.

If you use your own network protocol, you might be able to get "all" updates into CS-Studio, and with your suggested changes you can then send "all" updates to the widgets, but beware that you might just set yourself up to GUI freezes.

willrogers commented 3 years ago

OK, so presumably you are using this feature for some additional widgets that you have developed, is that right? But if you are still using Luna are you building from the master branch? Or do you have a fork? Is this a contribution for others in the collaboration, or do you need it in order to make progress.

If @kasemir is happy and it doesn't affect existing widgets then I will merge it, but I would like to understand the situation.

kasemir commented 3 years ago

I'm perfectly happy with introducing the fundamental support for 'queued' events in addition to the existing support for 'cache the latest value'. As I understand the current PR, all it does is offer that option without changing any existing widget and its behavior.

For the next step of actually using the new 'queue' option:

pedroramirezp commented 3 years ago

We have some new browser widgets that we configure from our code that need to handle each update. With this change, we tried to avoid the rest of the code would be affected.

Moreover, we aren't able to upgrade the CS-Studio version due to one of our requirements.

willrogers commented 3 years ago

I'm sorry, I missed the important detail that you want to merge this into the 4.1.x branch! I can be pretty sure that no-one else is using this branch any more, so it is fine to merge.

If you want to update to the master branch of this repo again, I suggest that you make another PR. If you can, moving to Phoebus would probably be an improvement some time in the future.