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

Reduce legacy Windows driver ANSI escape sequence intermediate string allocations #3936

Conversation

TheTonttu
Copy link
Contributor

@TheTonttu TheTonttu commented Feb 28, 2025

Fixes

Proposed Changes

  • Add new methods to EscSeqUtils that append directly to StringBuilder.
  • Use the new methods in Windows Console and Driver.

Pull Request checklist

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

Drivers

This should be applicable to the other drivers as well. I can of course make the changes if necessary but I have no WSL set up at the moment so I can only test the NetDriver.

UICatalog Benchmark

Average time for me improved from ~22 sec to ~21 sec +/- 1 sec.

Microbenchmarks


BenchmarkDotNet v0.14.0, Windows 10 (10.0.19045.5487/22H2/2022Update)
AMD Ryzen 7 5800X3D, 1 CPU, 16 logical and 8 physical cores
.NET SDK 8.0.406
  [Host]     : .NET 8.0.13 (8.0.1325.6609), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.13 (8.0.1325.6609), X64 RyuJIT AVX2

Method Mean Error StdDev Ratio RatioSD Gen0 Allocated Alloc Ratio
Set 162.82 ns 2.867 ns 2.394 ns 1.00 0.02 0.0026 136 B 1.00
Append 49.61 ns 0.905 ns 0.846 ns 0.30 0.01 - - 0.00

Allocations

Before

1 before allocations

1_1 escsequtils before allocations

After

2 after allocations

2_2 escsequtils after allocations

…diate string allocations

Appending InterpolatedStringHandler directly to StringBuilder skips the formatting related intermediate string allocation. This should also be usable in other console implementation but currently I have no WSL etc. setup to actually verify correct functionality.
@TheTonttu TheTonttu requested a review from tig as a code owner February 28, 2025 18:52
@tznind
Copy link
Collaborator

tznind commented Feb 28, 2025

The existing drivers should be replaced in due course by #3837 so may not be worth investing a lot of time optimising

But up to you. Changeover will only happen after period of both running side by side

@TheTonttu
Copy link
Contributor Author

The existing drivers should be replaced in due course by #3837 so may not be worth investing a lot of time optimising

But up to you. Changeover will only happen after period of both running side by side

Neat! I agree, seems a bit pointless to poke the current drivers any further then. Thankfully the new methods can also be utilized in the new drivers.

@TheTonttu
Copy link
Contributor Author

Grats, the new drivers were merged while I was writing. 🎉

@TheTonttu TheTonttu changed the title Reduce Windows driver ANSI escape sequence intermediate string allocations Reduce legacy Windows driver ANSI escape sequence intermediate string allocations Feb 28, 2025
@tig tig merged commit bc8bf38 into gui-cs:v2_develop Mar 1, 2025
4 checks passed
@TheTonttu TheTonttu deleted the windows-driver-reduce-ansi-escape-sequence-intermediate-string-allocations branch March 1, 2025 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants