Skip to content

Conversation

@skyline75489
Copy link
Collaborator

Summary of the Pull Request

Don't call flush when circling is triggered in VtRenderer.

References

Currently every time TriggerCircling is called, VtRenderer will force paint the frame and then flush the internal buffer. This is not really necessary because the max drawing frequency for graphic renderers like DxRenderer is limited. This causes an exceptionally bad case for the "1 million yes" benchmark: every time a “y" is printed, a single line if flushed, which is y(lot of spaces)\n. With this fix, VtRenderer will not flush the buffer eagerly and wait for more text. This improves the throughput.

PR Checklist

  • Supports [Performance] vtebench tracking issue #10563
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/passed
  • Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
  • Schema updated.
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

@skyline75489
Copy link
Collaborator Author

skyline75489 commented Jul 12, 2021

On my PC, the time needed for "1 million yes" dropped from ~8.0s to 6.2s in WT. For conhost it seems to worsen the performance a bit, because there's more pressure in the GDI renderer, which is heavily slowed down by #10528.

OpenConsole.exe trace

Before:

image

After:

image

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

I'm gonna temporarily block this while I look up in the archives why this is there. This might be secretly load bearing for something like telnet or something silly.

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 12, 2021
@zadjii-msft zadjii-msft removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 12, 2021
@zadjii-msft zadjii-msft self-assigned this Jul 12, 2021
@zadjii-msft zadjii-msft added Area-Performance Performance-related issue Issue-Task It's a feature request, but it doesn't really need a major design. labels Jul 12, 2021
@zadjii-msft
Copy link
Member

for my own reference, these are OS commits 900da83ab6934aad8c5d5d9688968ea7e634c563 and 9f89d02bbbce9283ec3a0fd6193e97f27cbcdb13

@skyline75489
Copy link
Collaborator Author

This actually slows down cat big.txt. Need to investigate a bit.

@skyline75489
Copy link
Collaborator Author

Yeah this slows down the catting of large files significantly. For example for a 100MB file the time needed increases from 20s to 30s. I still don’t quite understand the reason.

@skyline75489
Copy link
Collaborator Author

Closing this because it's not good enough and idk how to improve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Performance Performance-related issue Issue-Task It's a feature request, but it doesn't really need a major design.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants