Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a single renderer instance per window to reduce memory usage #15186

Open
lhecker opened this issue Apr 15, 2023 · 5 comments
Open

Use a single renderer instance per window to reduce memory usage #15186

lhecker opened this issue Apr 15, 2023 · 5 comments
Labels
Area-AtlasEngine Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Milestone

Comments

@lhecker
Copy link
Member

lhecker commented Apr 15, 2023

The problem: On my system, when Windows Terminal is maximized, every single DxRenderer/AtlasEngine instance consumes about 30MB committed virtual memory, 10MB physical memory, as well as 34 threads. Most of this is just due to the creation of the D3D11 device. It also consumes 100MB committed GPU memory, most of which is due to the creation of the swap chain. This cost quickly stacks up when you have a lot of open tabs. It gets more dire if you consider that a certain brand of stylish laptops are being sold right now with a total (across CPU and GPU) amount of memory of 8GB, but a 200+ PPI display.

The solution: Instead of creating one renderer per pane, we could create one renderer per window.

Proposed technical implementation details (optional)

Option 1 (?): https://learn.microsoft.com/en-us/uwp/api/windows.ui.composition.compositiontexture?view=winrt-26100

Option 2:

The swap chain would basically exist "behind" the XAML overlay that draws the pane grid lines. (Although, optimally, the renderer would draw them, so that DWM doesn't need to compose two swap chains.)
To allow us to draw multiple panes in a tab, we'll simply pass the renderer an array of IRenderData, one IRenderData per pane. The renderer will loop over the array and draw them iteratively into the swap chain at the pane's supposed position. Switching tabs simply switches the array of IRenderData that is being drawn. In case of AtlasEngine each IRenderData is assigned one IBackend instance which contains the glyph cache.
Implementing this will only be possible once IRenderData is less of a I and more of a Data, so that we can create multiple of them easily, pass them around, etc. It would also require us to implement buffer snapshotting and move the DirectWrite text segmentation into the IBackend. Afterwards, the above change can be implemented within a few days (I've already prepared most of what would be necessary for this).

A remaining open question is the overhead of switching tabs since the above proposal would lead to the destruction of the old IBackend array when the IRenderData array is being replaced. But I believe that this can be solved fairly easily by simply backing up the last used IRenderData/IBackend array, so that switching back and forth between 2 tabs is very quick at least (the most common scenario I can imagine). IBackend creation isn't that expensive though, so even switching between a third tab wouldn't lag that much.

@lhecker lhecker added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Apr 15, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Apr 15, 2023
@lhecker lhecker added Product-Terminal The new Windows Terminal. Area-AtlasEngine and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Apr 15, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Apr 15, 2023
@lhecker lhecker changed the title Use a single renderer instance per window Use a single renderer instance per window to reduce memory usage Apr 15, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 15, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Apr 15, 2023
@zadjii-msft zadjii-msft added this to the Backlog milestone Apr 17, 2023
@zadjii-msft
Copy link
Member

Something we should note - If we do chose to do something like this, it should be possible to be used by another application that wants to embed the Terminal as a TermControl (or whatever). Or it at least shouldn't prevent another app from just adding a <Terminal.TermControl /> to their app easily.

Maybe there's something like a RendererPool abstraction we could make once that would provide an encapsulation of all the DX resources that really only need to be instantiated once per-app, and then have each control share those? I dunno.

@rlabrecquefsg
Copy link

Hey guys, I found this ticket after debugging why I was maxed out on GPU memory, and just wanted to provide some details on what I was seeing since it /could/ be related.

I've had a Terminal window open with 6 tabs for like a week or two straight, and found that Windows Terminal (Version: 1.17.12191.0) was consuming 4GB worth of GPU memory while idle.
Other notes: I've accessed this machine via RDP a bunch, Windows has been been locked/unlocked many times, the terminal window has been minimized, I've switched terminal tabs a bunch, run many short lived processes with it. Primarily was using powershell + WSL. My terminal isn't configured much.

You can see the moment I closed Windows Terminal in this screenshot:

image

@lhecker
Copy link
Member Author

lhecker commented Sep 13, 2023

Do you use AtlasEngine? The maximum expected GPU memory for non-AtlasEngine is ~256MB per text renderer instance + window area in pixels * 2. For AtlasEngine the max. memory usage in stable 1.17 is ~64MiB + window area in pixels * 2. Both limits are only reached after encountering a ton of Unicode and Emojis.

If you see such a significant memory usage with just 6 tabs, I think it's somewhat likely that you're seeing either a memory leak in the driver (Nvidia is primarily focused on how creating 1 D3D device per app lifetime works - just like in games, but we create and close D3D devices once per tab/pane), or it's leaks in WinUI, which I think is fairly likely. We had related issues with WinUI before as well (due to animations not completing when the system sleeps).

If you'd like you could install Windows Terminal Preview (v1.18) which has an overhauled text renderer that may happen to fix that leak (if it was a leak in the text renderer after all). In particular, it allows you to reliably release any and all resources it holds if you enable "software rendering" in the "Rendering" section of the settings dialog (as usual, you need to click Save though). Switch the setting on, navigate to each tab at least once (to make sure it went through a refresh) and then just toggle it off again. This will destroy and recreate all D3D devices and resources. If it still holds onto GPU memory at that point it's definitely either the graphics driver or WinUI.

@zadjii-msft
Copy link
Member

Notes from team sync

it may be easier to write a d3d12 renderer than doing #1989 (because we'd need a renderer pool of some sort, because creating a d3d device takes too long to create them every time you switch tabs)

But also probably the most correct solution is #15186.


We're gonna leave this open for now, because we're probably going to see more reports of this soon.

@lhecker
Copy link
Member Author

lhecker commented Jul 30, 2024

We can possibly simply use this XAML composition class instead: https://learn.microsoft.com/en-us/uwp/api/windows.ui.composition.compositiontexture?view=winrt-26100

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Tag-Fix Doesn't match tag requirements label Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AtlasEngine Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

3 participants