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

Cache the viewport to make invalidation faster #6918

Merged
5 commits merged into from
Jul 16, 2020

Conversation

miniksa
Copy link
Member

@miniksa miniksa commented Jul 14, 2020

In Renderer::TriggerRedraw, the act of fetching the viewport from the
pData over and over is wasted time. We already have a cached variable
of the viewport that is updated on every scroll check (on
TriggerScroll and on PaintFrame.) Scrolling wouldn't be working
correctly if the clients weren't already notifying us that the viewport
has changed for scroll purposes, so we can just keep using that cached
value for the invalidation restriction to speed things up over fetching
it again.

Validation Steps Performed

  • Run time cat big.txt. Checked average time before/after, WPR traces
    before/after.

PR Checklist

  • Closes perf itch
  • I work here
  • Manual test
  • Documentation irrelevant.
  • Schema irrelevant.
  • Am core contributor.

@miniksa miniksa added Area-Performance Performance-related issue Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. labels Jul 14, 2020
@miniksa miniksa self-assigned this Jul 14, 2020
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh. We were caching it, but not using the cached value? Classic us.

@miniksa miniksa added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jul 16, 2020
@ghost
Copy link

ghost commented Jul 16, 2020

Hello @miniksa!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 81b7e54 into master Jul 16, 2020
@ghost ghost deleted the dev/miniksa/perf_viewport_cache branch July 16, 2020 21:46
@ghost
Copy link

ghost commented Jul 22, 2020

🎉Windows Terminal Preview v1.2.2022.0 has been released which incorporates this pull request.:tada:

Handy links:

DHowett added a commit that referenced this pull request Aug 5, 2020
Carlos Zamora (1)
* UIA: use full buffer comparison in rects and endpoint setter (GH-6447)

Dan Thompson (2)
* Tweaks: normalize TextAttribute method names (adjective form) (GH-6951)
* Fix 'bcz exclusive' typo (GH-6938)

Dustin L. Howett (4)
* Fix VT mouse capture issues in Terminal and conhost (GH-7166)
* version: bump to 1.3 on master
* Update Cascadia Code to 2007.15 (GH-6958)
* Move to the TerminalDependencies NuGet feed (GH-6954)

James Holderness (3)
* Render the SGR "underlined" attribute in the style of the font (CC-7148)
* Add support for the "crossed-out" graphic rendition attribute (CC-7143)
* Refactor grid line renderers with support for more line types (CC-7107)

Leonard Hecker (1)
* Added til::spsc, a lock-free, single-producer/-consumer FIFO queue (CC-6751)

Michael Niksa (6)
* Update TAEF to 10.57.200731005-develop (GH-7164)
* Skip DX invalidation if we've already scrolled an entire screen worth of height (GH-6922)
* Commit attr runs less frequently by accumulating length of color run (GH-6919)
* Set memory order on slow atomics (GH-6920)
* Cache the viewport to make invalidation faster (GH-6918)
* Correct comment in this SPSC test as a quick follow up to merge.

Related work items: MSFT-28208358
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Performance Performance-related issue AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants