Piston Backend: Render/Input event starvation when Update events consume too much CPU time

[christolliday]

This is related to #824

When running in debug mode, or on a slower machine, or when an app is complex enough that the update loop consumes 100% of the CPU, the pistoncore-event_loop continues posting Update events while the application starves for Render and Input events.
This happens in debug mode in any of the examples that have more than a few widgets on my 3 year old i7 (thinkpad t440s).

3797920 removed set_ups(60) from the examples, which makes those examples use the default update rate of 120 updates per second (updates per second is different from frames per second in pistoncore-event_loop). This makes the issue appear more easily, but it happens with or without set_ups(60).

[mitchmindtree]

removed set_ups(60) from the examples, which makes those examples use the default update rate of 120

Ahh my apologies, I removed the set_ups calls with the assumption that it would match the default fps.

@christolliday I'd love to get your thoughts on the glutin_glium.rs example. It makes me wonder whether this work adapting and maintaining the piston event loop and window abstractions is worth it when the glutin loop + polling seems to achieve what we're going for already in a simpler fashion with less indirection and more control in the hands of the user? By more control I mean:

• They can setup their own timed loops however they wish without the need for lots of trait implementations (as is the case in our piston/window backend)
• They can poll or wait for all queued input events at once
• They don't need to reach through a wrapper to access the window

I imagine this might be the case for users wanting to use conrod with other window managers like glfw and sdl2 as well.

I guess the more I think about it, the more I'm unsure of what benefits we are gaining by maintaining an implementation of the traits within the piston event and window libraries?

One thing that I do believe is useful to conrod is the conversion of different backend events to a common Input type, however as demonstrated with conrod's glutin feature this could be achieved with a single conversion function, rather than wrapping the entire window as is the case in glutin_window, sdl2_window, etc.

I'm curious to get your thoughts on all this.

[christolliday]

Some good questions. I think most apps aren't going to need custom logic for polling/waiting, so I think it is useful to have some kind of event loop, separate from the app main loop that is reusable, while still allowing apps to easily use their own. I'm not sure how that can be done without some amount of indirection, unless we only support glutin, but I do agree with you about there being too much indirection in the piston backend.

In the glutin_* examples, the loop of doing everything and waiting 16ms is better than the default piston loop and fixes the render event starvation issue, but it still does extra work by polling when idle and could respond slower than a loop that waits for events, depending on the timing and if the update loop takes > 16ms. It's the indefinite waiting that introduces some complexity, in the context of Conrod, but I think that part is important.

Also, it seems like as long as there is value in having different backends, there is value in having code that is generic over the different backends, and the backend surface area is more than just the events that come out. For example, for an event loop, the piston Window trait is useful, to call poll_event and wait_event on both glutin and sdl2. Also there can be generic window graphics code, like GfxContext, that can just depend on piston's OpenGLWindow. Other than that though, I don't think there are other traits needed. Also there's no dependency on the piston event crate needed if you were wondering.

In terms of the glutin backend though, I don't know whether that will eventually become default and then being generic over other windows will become less of a priority? Then the event loop etc. could just operate on glutin::Window directly.

In terms of removing indirection right now, I actually think it makes sense to remove the piston::backend::Window struct that I just added. Since I refactored out the graphics part, the window itself is just a thin layer of indirection to maintain most of the old piston_window API. I think it's more straightforward to just have clients create a window (GlutinWindow etc.), event loop, and graphics context (eg. GfxContext) manually.

Same for the trait that binds an event loop to a window. It's only needed to call through to the window's handle_event method automatically. I think it's just as easy to have apps just call handle_event on the window themselves, and it would make event handling less confusing in general.

[mitchmindtree]

In the glutin_* examples, the loop of doing everything and waiting 16ms is better than the default piston loop and fixes the render event starvation issue, but it still does extra work by polling when idle and could respond slower than a loop that waits for events, depending on the timing and if the update loop takes > 16ms. It's the indefinite waiting that introduces some complexity, in the context of Conrod, but I think that part is important.

Totally agreed with this - in a more serious application like a game or real-time media app I'd expect the user would use something better designed for this, whether that was a piston event loop or something else. If we do want to use something slightly more accurate in the conrod examples, I imagine we could just provide a function in the examples/support module kind of like how glium does.

I think it's more straightforward to just have clients create a window (GlutinWindow etc.), event loop, and graphics context (eg. GfxContext) manually.

Same for the trait that binds an event loop to a window. It's only needed to call through to the window's handle_event method automatically. I think it's just as easy to have apps just call handle_event on the window themselves, and it would make event handling less confusing in general.

Absolutely, this is the way I've been leaning too. Even if the piston_window-esque API saves us a couple lines in the examples, I don't think it is providing nearly enough benefit to maintain it when setting up windows manually as you mention is almost as easy.

We've already received an issue that suggests separating the piston feature into separate parts - I think if we continue down this track, it could easily lead to a can of worms where we end up with loads of segregated piston features which require matching versions, making stabilising tricky and adding more required maintenance for little benefit.

With regards to the piston backend feature, I'm starting to think that all we should really need to maintain are the draw and event modules, as these are unambiguously useful across almost all combinations of piston graphics+window+event combinations that would like to use conrod. I think we'd do best to leave any other higher-level window and event loop stuff to the other piston crates at least for now as there's still so much other work conrod needs in order to approach stabilising.

How do you feel about the idea of stripping down the piston feature to only the draw and event modules that I mention above? This would of course involve updating all of the examples, whether we use a GlutinWindow+Gfx piston combo like you mention or whether we switch them over to the glutin+glium features. I think my personal preference would be switching them to the glutin+glium features, primarily as they're very simple, non-generic, pure-rust and performing much better than the piston feature at the moment.

[bvssvni]

Notice that if update events are lagging you should do one of the following:

• Use your own timer
• Off-load work to the idle event

The event loop in the Piston core is designed to be used this way for games. In applications there is a different usage pattern. Perhaps we could add something to the docs to make this clearer?

I completely agree that one should use as little API surface as possible. This was the intention of the Piston core. Depending on PistonWindow is an anti-pattern for libraries. I think PistonWindow is for quick prototyping and game jams, and should be thought twice about when used in applications. People have grown accustomed to it because of the extra setup to use Gfx, but this might get better someday (I hope). Don't depend on it in libraries! (@mitchmindtree please pay attention to this, I think I mentioned it 5 times already).

I do not recommend dropping backend agnostic design in favor of simpler dependency graph at this point. There is no guarantee that design choices in Glutin will hold up for future hardware, but the Piston core is designed with that intention. There are also small differences that makes a specific window backend a better choice, for example better mouse cursor coordinates in Glfw for painting tools.

[mitchmindtree]

(@mitchmindtree please pay attention to this, I think I mentioned it 5 times already).

I'm aware of this and I'm not sure why you are implying that I am not.

I do not recommend dropping backend agnostic design in favor of simpler dependency graph at this point.

Conrod will remain agnostic of the window backend - when I mentioned the possibility of switching to use glutin for simplicity I was talking about the examples, sorry if that wasn't clear.

[bvssvni]

@mitchmindtree I might have misunderstood your comment:

With regards to the piston backend feature, I'm starting to think that all we should really need to maintain are the draw and event modules, as these are unambiguously useful across almost all combinations of piston graphics+window+event combinations that would like to use conrod. I think we'd do best to leave any other higher-level window and event loop stuff to the other piston crates at least for now as there's still so much other work conrod needs in order to approach stabilising.

I thought Conrod depended on PistonWindow instead of the Piston core. My mistake.

[christolliday]

How do you feel about the idea of stripping down the piston feature to only the draw and event modules that I mention above? This would of course involve updating all of the examples, whether we use a GlutinWindow+Gfx piston combo like you mention or whether we switch them over to the glutin+glium features. I think my personal preference would be switching them to the glutin+glium features, primarily as they're very simple, non-generic, pure-rust and performing much better than the piston feature at the moment.

I think the piston backend should be reduced to the draw and event modules and the events event loop module in PR right now. The gfx part should be separated out into backend::gfx as proposed here. That leaves just the window part to be removed, as mentioned above.

I don't see why the examples can't be switched over to glutin+glium backends once they provide a similar level of abstraction. For me piston and gfx is working well enough right now, but I understand if you want to focus on a single backend for maintenance reasons, you're probably right that glutin/winit is the best solution long term, but maybe the piston window code can be retained as non-default in case glutin doesn't work everywhere. In terms of glium vs gfx, it also seems reasonable to have multiple options. As long as piston is the default though, it makes sense to keep improving it, and any fixes there can't hurt development in other backends.

We've already received an issue that suggests separating the piston feature into separate parts - I think if we continue down this track, it could easily lead to a can of worms where we end up with loads of segregated piston features which require matching versions, making stabilising tricky and adding more required maintenance for little benefit.

I see what you mean, but I think as long as all the piston dependencies are only specified by the backend it shouldn't be an problem? Also, the separation proposed there is only about separating non-piston parts, ie. gfx, so shouldn't cause that kind of issue.

If we do want to use something slightly more accurate in the conrod examples, I imagine we could just provide a function in the examples/support module kind of like how glium does.

Maybe the issue can be solved by using different timing or changing the piston event loop, but it's better resolved by just not using the piston event loop and allowing the event loop to go idle. What are your thoughts on the waiting event loop here? I think it solves problems right now, both in terms of bugs and giving a better impression of performance.

If you're worried about it becoming useless if piston is dropped, I think the same logic and API can be reused, just that we could port the piston events Render, AfterRender and Update (or maybe only Render) to conrod events, and get events from a winit/glutin window instead of GlutinWindow, so it is easily ported. In fact I think it should be once winit/glutin becomes default. It could probably also be adapted to provide a futures/callback based api if winit goes in that direction.

Anyway, I probably won't have much free time to work on it for about a month, but I could look into it then, and maybe do some work on the glutin+glium backends in general at that point.

[christolliday]

Sorry looking at this issue I realize it's actually about separating draw and piston-graphics out, not gfx, but I still think having issues with matching versions seems unlikely, since they are all still defined in one Cargo.toml. Anyway I think the gfx part can be put in backend::gfx and potentially be used by both piston examples and glutin+gfx.

[mitchmindtree]

For me piston and gfx is working well enough right now, but I understand if you want to focus on a single backend for maintenance reasons

If you're worried about it becoming useless if piston is dropped, ..

Just to clarify, I have no intention of dropping the piston feature, I only want to trim it down to the essential draw and event modules. I only wish to switch the examples to the glutin/glium backend for the reasons I mentioned previously and elaborated on a little in this comment.

Maybe the issue can be solved by using different timing or changing the piston event loop, but it's better resolved by just not using the piston event loop and allowing the event loop to go idle. What are your thoughts on the waiting event loop here? I think it solves problems right now, both in terms of bugs and giving a better impression of performance.

I'll reply to this at #871 as it is probably a better place to discuss it 👍

just that we could port the piston events Render, AfterRender and Update (or maybe only Render) to conrod events

Keep in mind that conrod doesn't need to know about any of these events anymore - Ui::handle_event only takes Input events as of #855.

[mitchmindtree]

Going to close as this is more a problem with the limited availability of GUI-compatible piston event loops rather than an issue in conrod itself. See some related discussion and further links at #909.

[bvssvni]

@mitchmindtree The Piston core has made changes that now makes it better for GUI.