Skip to content

Avoid querying for cursor position when it's not necessary #4448

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 4, 2025
Merged

Avoid querying for cursor position when it's not necessary #4448

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
90d6959
Avoid querying for cursor position unnecessarily
daxian-dbw Jan 7, 2025
0c0c39f
Skip unnecessary window resizing check to avoid cursor position queries
daxian-dbw Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions PSReadLine/Render.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,14 @@ public void UpdateConsoleInfo(IConsole console)
cursorLeft = console.CursorLeft;
cursorTop = console.CursorTop;
}

public void UpdateConsoleInfo(int bWidth, int bHeight, int cLeft, int cTop)
{
bufferWidth = bWidth;
bufferHeight = bHeight;
cursorLeft = cLeft;
cursorTop = cTop;
}
}

internal readonly struct RenderDataOffset
Expand Down Expand Up @@ -212,9 +220,9 @@ private void RenderWithPredictionQueryPaused()

private void Render()
{
// If there are a bunch of keys queued up, skip rendering if we've rendered
// recently.
if (_queuedKeys.Count > 10 && (_lastRenderTime.ElapsedMilliseconds < 50))
// If there are a bunch of keys queued up, skip rendering if we've rendered very recently.
long elapsedMs = _lastRenderTime.ElapsedMilliseconds;
if (_queuedKeys.Count > 10 && elapsedMs < 50)
{
// We won't render, but most likely the tokens will be different, so make
// sure we don't use old tokens, also allow garbage to get collected.
Expand All @@ -225,6 +233,20 @@ private void Render()
return;
}

// If we've rendered very recently, skip the terminal window resizing check as it's unlikely
// to happen in such a short time interval.
// We try to avoid unnecessary resizing check because it requires getting the cursor position
// which would force a network round trip in an environment where front-end xtermjs talking to
// a server-side PTY via websocket. Without querying for cursor position, content written on
// the server side could be buffered, which is much more performant.
// See the following 2 GitHub issues for more context:
// - https://github.com/PowerShell/PSReadLine/issues/3879#issuecomment-2573996070
// - https://github.com/PowerShell/PowerShell/issues/24696
if (elapsedMs < 50)
{
_handlePotentialResizing = false;
}

ForceRender();
}

Expand Down Expand Up @@ -928,7 +950,7 @@ void UpdateColorsIfNecessary(string newColor)
_console.SetCursorPosition(point.X, point.Y);
_console.CursorVisible = true;

_previousRender.UpdateConsoleInfo(_console);
_previousRender.UpdateConsoleInfo(bufferWidth, bufferHeight, point.X, point.Y);
_previousRender.initialY = _initialY;

// TODO: set WindowTop if necessary
Expand Down