Performance of the main loop

[ashinkarov]

It seems to me that the current design of the main loop makes wastes a lot of time constantly checking things without any delays or pauses. Here is my thinking.

Try to paste some text in profanity. Well, you would be able to observe, that it takes considerable amount of time, and actually you can note how long it takes to process each character. Now, if you will look in top/htop, you will see that it constantly takes 1-1.5% of a CPU. That is not ok. My X server takes less than that.

So, further investigation revealed the fact that actually nested event-loop goes without any pauses and constantly redraws all the ui, checks the timers, runs the plugins, etc. Which certainly explains huge delays.

My proposal would be to split the events of the min loop into user-input and interface events and run them in two different threads. That would speed-up input and would allow to put reasonable delays between the ui and timing events. Also, I would suggest that the input-loop should be based on getch-like functionality rather than on wget_wch, which would make it do something only if a user actually pushed some button. Also, while typing, we don't need to update all the interface I would think, we need to update the input line only.

Any comments?

[alandrees]

As a daily user of Profanity, I can attest to this. When pasting links and the like into my profanity session, I can see the whole screen redrawing.

I haven't really taken too deep a look at the source yet, however since the UI is heavily based on Irssi, why not incorporate some of their UI drawing techniques?

[boothj5]

The main event loop could certainly do with some reworking and is something that has been put off for a while, whilst other features have been added.

Threading some tasks would help a lot, currently for example the /rooms command when connected to jabber.org, locks the whole UI whilst the large number of rooms are being retrieved and processed, there's no reason why this shouldn't be happening in the background whilst the user continues doing other things.

The input handling is also currently a very manual and complex process (input_win.c), character by character. If the input line was separated from the main loop as you suggest, it might also be easier to use something like GNU readline, which has been suggested, and would only interrupt the main loop when there is something to do with the line.

This will be part of the focus for the 0.4.0 release, any suggestions/patches very welcome.

[pasis]

Redesign is a big amount of work and can lead to some problems. We should think about impact of separating to threads:

• Logging subsystem provides functionality for all threads (for libstrophe too) thereby must be thread-safe
• Libstrophe single-thread and this must be taken into account
• The same with UI
• Relations between threads should be minimized to reduce using of synchronization
• etc

So I suggest to make high-level design and then detailed before actual implementation.

[boothj5]

I would definitely agree with that. One of the biggest reasons for not doing it so far is the additional complexity involved.

If anyone has thoughts, or design suggestions, please post them here. We'll avoid making any changes to the code (in master at least) until/unless there is a clear and safe way forward.

[savar]

So one step in between (before doing it really "right") could be dual strategy. We know that we can reduce a lot of CPU consumption if we would block a longer time but this also introduces a lag (like the timeout of 20ms is doing right now)..

I thought about something like having an increasing timeout instead of a static one.. something like blocking 10x without a delay, then 10x with 2ms, ... and at the end block for 500ms (or the specified inpblock).. and if any of the "ui events" functions (src/ui/ui.h:ab75059:118-241) is triggered the timeout counter is set back to 0 and also the act_timeout_ms value...

We may be able to introduce also another timeout which would help us in not drawing the main window on each event so that a copy'n'paste would be faster.. something like "only do an ui_update after 10 'timeouts' without any ui events or latest after max_wait_timeout (like 50ms)"...

[boothj5]

Until an event driven approach is implemented, @savar's suggestion has been implemented with #483

As a result of the change, the UI remains responsive to input, and the CPU usage quickly reduces to close to 0% after no activity.

The two downsides to this approach are:

1. When the user is not interacting with profanity, messages might be displayed in the UI with a delay up to the timeout value (maximum 1 second, default 500ms), after profanity receives them.
2. CPU usage can rise to 100% when pasting large amounts of text

Since the delay in 1) is small, and the large pasting is a separate issue that needs to be dealt with, this solution seems better than the previous fixed timeout. However, should users prefer, they can still go back to the old behaviour using a preference.

[weiss]

@ashinkarov

My proposal would be to split the events of the min loop into user-input and interface events and run them in two different threads.

Why are separate threads required for this? Can't both be handled through a single event loop?

[ashinkarov]

It is not really a requirement. It is just that a number of ui actions are independent from any data exchange. I.e. you can keep scrolling/switchin windows, etc., while polling on a slow connection, or processing a big input. It is surely possible to run everything on a single thread, but one might end up with quite a complex state within the event loop. Separating ui updates and data exchanges seem to be a common practice. However, I surely wouldn't fight for it :)

Keep in mind that I wrote this comment 6 years ago, and I haven't used Profanity since then, so I have very little feel about the state/complexity of the code.

[Glandos]

@boothj5 commented on 13 janv. 2015, 01:11 UTC+1:

1. When the user is not interacting with profanity, messages might be displayed in the UI with a delay up to the timeout value (maximum 1 second, default 500ms), after profanity receives them.

Even this delay eats up much more CPU than needed. I am currently evaluating both poezio and profanity, and based on a idling client, poezio eats 60× less CPU, due to its use of Python asyncio. But maybe it's too much asking to have a event loop based on events with a epoll mechanism?

[H3rnand3zzz]

Partially resolves #1943