Bad window resize performance with large READMEs on some systems

[CosmicHorrorDev]

For some reason on my system resizing the window has really really bad performance

Demo

https://user-images.githubusercontent.com/30302768/185724443-d6ed6c2c-0760-48cf-ad69-a46e35a53f8d.mp4

As you can see it processes 100+ events for WindowEvent::Resized that all take ~20ms along with one beefy event for Event::RedrawRequested that takes ~16s

If I do small shifts to the window then it's not a problem. It's only when a lot of WindowEvent::Resized get queued up that Event::RedrawRequested takes a long time

What's the CPU usage look like?

It takes up all of one logical core for the whole duration

Memory usage?

Peak usage is usually over 10 GiB :smiling_face_with_tear:

This does not go down after the resize finishes

What if you do it twice?

The WindowEvent::Resizeds consistently each takes ~20ms while the Event::RedrawRequested takes around 1.5 seconds

Is this a recent problem?

Nope. I can reproduce it on commits back before I even started contributing

Does this happen with smaller READMEs?

No! The issue seems to only exist when the README is big enough that another WindowEvent::Resized gets queued up before the last one finishes

Profiles

Both profiles are from the same situation shown in the video aka one resize and then killing the program

CPU usage

As you can see the vast majority of the time is spent in glyph cache related things

Memory usage

Note: 12.3GB peak memory usage

Unsurprisingly also dominated by glyph cache related things

Investigation

The vast majority of the time for the WindowEvent::Resized related calls are taken up from this .glyph_bounds() call

https://github.com/trimental/inlyne/blob/14422b0d3a2a7893ee483df8b5c6ae9fc239d4e9/src/text.rs#L100

The vast majority of the time for the Event::RedrawRequested call is taken up by this .draw_queued_with_transform() call

https://github.com/trimental/inlyne/blob/14422b0d3a2a7893ee483df8b5c6ae9fc239d4e9/src/renderer.rs#L572-L597

Two things both reduce the time taken for Event::RedrawRequested to ~1.5s and reduced the peak memory usage from ~12GB to ~4.5GB

1. Adding .draw_cache_position_tolerance(1.0) to the glyph brush builder (This ignores sub-pixel position changes when caching glyphs)
2. Setting the scale to 1.0 (My hunch here was that the hidpi_scale was inconsistently used somewhere causing duplicate glyphs to be added to the cache)

And sadly I haven't found anything that makes WindowEvent::Resized process faster. I think filtering consecutive queued WindowEvent::Resized events could help since there is no reason to process a resize if there is another resize already queued after it. I did a hack to only process every 10th WindowEvent::Resized which made things refresh smoothly aside from having obvious graphical issues. Changing the event loop to allow inspecting queued up events would be very non-trivial though. My best idea on how to handle it would be to send events over a channel to another thread that actually processes them, but this involves a lot of changes from lifetimes, callbacks, etc.

All of these seem like they're trying to work around some underlying issue that I can't find though

Pending questions

Does something queue up each time there is a WindowEvent::Resized? It seems that way since the time taken up by Event::RedrawRequested depends on the number of WindowEvent::Resized that have run before it

What's causing the glyph cache to blow up so much? Even with the hack above 4.5 GB peak memory usage still seems very high

[CosmicHorrorDev]

May have spoke too soon on how to handle inspecting the event queue. I have an idea on how to handle the window resizes better after reading more of winit's docs

[trimental]

Jesus 12.3 gb is really bad 😢. My first thought is that this must be some sort of leak in wgpu glyph, this definitely needs to be looked into! Did you manage to find the inconsistent usage of hidpi_scale?

Btw like I commented in https://github.com/trimental/inlyne/pull/26 I made a branch that run the renderer asynchronously however I ran into issues with resizing. I think I might start looking into that approach again. Basically the only thread safe objects you would need are the element_queue which already is, and something like a crossbeam channel to send redraw and reposition requests to the renderer. Although if you can make https://github.com/trimental/inlyne/pull/26 work then perhaps this won't be needed 😄

[trimental]

Actually I lied, Positioned would have to be thread safe as well to handle things like managing clicks, scrollbar, etc

[CosmicHorrorDev]

Jesus 12.3 gb is really bad :cry:

Yeah it's a little rough lol. I think the window resized behavior is an X11 related issue based on semi-relevant issues I found. I'll have to setup VMs for some other X11 and wayland issues to see how it ends up behaving

Did you manage to find the inconsistent usage of hidpi_scale?

I didn't although I didn't spend very long looking

[trimental]

Yep I know what the memory issue is caused by. On macOS resize functions are immediately followed by redraw functions, which call draw_queued_with_transform which calls process_queued which trims the glyph cache if the cache for a glyph wasn't previously used in the last redraw queue.

Because we used this cache for getting the dimensions and bounds of text, every time we reposition we are adding to this cache and are not trimming it until the next redraw. On your system where there are multiple resize events before a redraw event, it leads to the cache storing the glyphs of every size before the redraw. If I remove request_redraw from the resizing event handler things get big quick!

[CosmicHorrorDev]

Ahhhh, that makes a lot of sense. I was wondering what part of the cache kept growing. I didn't realize that rendering trims the font cache

Thanks for clearing that up!