parro-it/libui-node

startLoop instead of uiMain

Closed this issue · 20 comments

First of all: This binding works and looks amazing. I am especially excited about your idea of a React-like interface (see sindresorhus/project-ideas#72), I would even plan to try it out on my own.

Anyway, when running the histogram example, I noticed that the node process uses 100% of one of my cores which is pretty intense for a simple window without interacting with it. It seems that your startLoop implementation consumes a lot of CPU time due to its recursive invokation. I replaced the call to startLoop, in the example, with the direct binding to uiMain and it still worked perfectly with much less consumption:

    mainwin.show();

    //libui.startLoop();
    libui.Ui.main();
}

Now, since I am not very familiar with libui, I was wondering what the differences between your startLoop and the uiMain approach are and why one would choose startLoop.

Thank you @Acconut. I wrote some documentation regarding main and startLoop here.

TLDR: you want to use startLoop because main is a blocking call and so Node.js event loop stop running until call quit method.

But maybe the recursion is to aggressive because I use setImmediate, I'll do some experiments trying to use process.nextTick or setInterval, it should help reduce CPU consumption.

Using setTimeout instead of setImmediate to schedule next GUI event loop tick drop CPU usage from 100% to ~5% on my machine. Please check it out and confirm if this solve the issue...

Using setTimeout instead of setImmediate to schedule next GUI event loop tick drop CPU usage from 100% to ~5% on my machine. Please check it out and confirm if this solve the issue...

I was able to reproduce the same, setTimeout seems to work great. However, I am not sure whether that's due the fact that it has a lower frequency than setImmediate. Anyway, it works better and that's the point.
Another possibility, I would like to try out, is to invoke the uiMain function in a new thread which may be created using libuv.

I was able to reproduce the same, setTimeout seems to work great. However, I am not sure whether that's due the fact that it has a lower frequency than setImmediate.

It is indeed. The counterpart is that doing so we slow down UI responsivity because we are processing libui events less frequently...

Another possibility, I would like to try out, is to invoke the uiMain function in a new thread which may be created using libuv.

@Acconut that would be a great feature to have, feel free to provide a PR!
But it won't be an easy path I guess...

Some suggestions:

  • Probably you'll have to use uiQueueMain libui function to defer functions execution to libui main thread.
  • Checkout the libui multithread example
  • How we can send libui methods call from the Javascript thread to the libui one? One by one is probably too slow, maybe grouped in a function?

It is indeed. The counterpart is that doing so we slow down UI responsivity because we are processing libui events less frequently...

While this is theoratically true, I do not believe this matters in practice. I added a very simple approach to obtain the frequency at which the libui events are processed, using this code:

function startLoop(cb) {
    var counter = 0;

    function step() {
        counter++;
        Ui.mainStep(false);
        if (loopRunning) {
                        // Replace it with setImmeditate or whatever
            setTimeout(step);
        } else if (cb) {
            cb();
        }
    }

    if (loopRunning) {
        return;
    }

    setInterval(function() {
        console.log(counter + "Hz");
        counter = 0;
    }, 1000);

    loopRunning = true;
    Ui.mainSteps();
    step();
}

The results are very interesting:

  • setImmediate runs at about 160kHz which is awfully often.
  • setTimeout(0) at usually 700Hz.

This will also explain the drastically reduced CPU usage. Anyway, since even 700Hz is unnecessary load and the recommended framerate (for games and websites) is 60Hz, I started to use setTimeout(step, 1000 / 60) locally since it's enough to provide a responsive UI at 60FPS while keeping my CPU cool.

Regarding the multithreaded approach: I attempted to do it, but ran into segmentation faults pretty soon (without putting much effort into this). However, I do think that we will not need these kinds of advanced features right now, or do you?

Really interesting approach @Acconut. I tried your code and get similar frequency.
~800Hz with setTimeout(0) and ~60KHz with setImmediate.

But I got strange behavior using setTimeout when I run node examples/control-gallery.js and If I click on menu -> Save.

This menu open a save as dialog, and when you choose file or cancel, it open a message box. Using setTimeout the message box seems to be freezed when I click ok, it close after a second or two.

After that the frequency drop to 0Hz for some time, the OS signal that the window is unresponsive and offers to kill the process...

Same stuff work well when using setImmediate. I have to do more investigation, I don't understand what may be causing this.

Regarding the multithreaded approach: I attempted to do it, but ran into segmentation faults pretty soon (without putting much effort into this).

I suspected it could have been difficult to implement 😃

However, I do think that we will not need these kinds of advanced features right now, or do you?

I agree with you. It probably greatly complicate all the code. It could be useful, but maybe in future release

Something like requestAnimationFrame?

@forivall: AFAIK @andlabs is implementing something similar on libui . Details are here

The libui event loop is also discussed here

@Acconut I solved the issue above by calling setImmediate one time after calls to msgBox. The actual code is on loop-investigation branch. Sadly, I don't underastand why this work, but it work 😢

Also, if I run with setTimeout at 60Hz, I notice that the UI is less responsive (E.g. menu items mouse hover effect is slow). With setTimeout this doesn't happens. CPU usage does not change between the two settings.

Yesterday I implemented a version of the event loop based on libuv polling.

Source code are on loop-investigation.

It work only on linux by now, but result are excellent: CPU usage drop from ~5% to ~0,6% and the UI is much more responsive too.

I tried it out on Linux as well, I didn't notice any issues with it and CPU usage is definitely down. If you want I can test it on Mac/Windows as well.

Thank you @PaulBGD, please do it.
But Windows is not yet supported: I have to prepare event loop code for it and setup build scripts.

Please re-test also on Linux, I reimplemented the event loop to use a separate libuv thread to check for events. It seems event more performant on my development machine, and the semantic is the same I'm using on macOS

Running in Arch Linux I get:
10-12% CPU usage on master

Turns out I can't compile the loop investigation on here since I'm missing a glibconfig.h which is in /usr/lib/glib-2.0/include

Running in macOS I get:
7-10% CPU usage on master
0.3-0.5% CPU usage on loop-investigation

@parro-it Your approach for event polling is really interesting and I am pumped to try it out next week! Thank you a lot for this hard work.

Thank you @PaulBGD , I'll check for the missing header file...
@Acconut : you're welcome!

@PaulBGD: I removed manually specified include dirs from binding.gyp file and used pkg-config command to auto-generate them at build time.

It should work on arch linux now. Could you give it a shot? Thank you

Compiles great now, I'm seeing 0.0-0.7% CPU usage.

Great! I will merge the branch into master soon and push a new version on npm

Ok, version 0.11 published on npm that contains event loop enhancement, going to close this issue.
Thank you all for your help 😺