Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

asking terminal for dimensions during every frame is expensive - #11504

#11504
Merged
SimaTian merged 3 commits into
maindotnet/msbuild:mainfrom
terminal_logger_dimensions_perfdotnet/msbuild:terminal_logger_dimensions_perfCopy head branch name to clipboard
Mar 11, 2025
Merged

asking terminal for dimensions during every frame is expensive#11504
SimaTian merged 3 commits into
maindotnet/msbuild:mainfrom
terminal_logger_dimensions_perfdotnet/msbuild:terminal_logger_dimensions_perfCopy head branch name to clipboard

Conversation

@SimaTian

@SimaTian SimaTian commented Feb 26, 2025

Copy link
Copy Markdown
Contributor

Part of #11160

Context

While profiling some of Eric's PRs and one of my experiments, I've noticed following:
terminall_loger_get_width_height_cost
Terminal width/height are behind a lock and repeated access is quite expensive.

Changes Made

I've set it so that the update is happening only once every ~second or so. This makes the cost negligible.
Note that this appears to be ~1.5% CPU time saved on the main node, which is the one under heaviest load due to IPC with all the other nodes.

Testing

None, this should be non-disruptive change.

Notes

Refresh frequency is up to discussion. Making it twice a second should be fine as well.

Comment thread src/Build/Logging/TerminalLogger/TerminalLogger.cs
@SimaTian
SimaTian requested a review from baronfel March 4, 2025 08:04
@SimaTian
SimaTian requested a review from MichalPavlik March 11, 2025 15:14
@SimaTian
SimaTian merged commit 0427e85 into main Mar 11, 2025
@SimaTian
SimaTian deleted the terminal_logger_dimensions_perf branch March 11, 2025 15:17
This was referenced Jul 15, 2026
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.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.