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

Integrated Terminal Lags Intermittently #105446

Closed
davidzech opened this issue Aug 26, 2020 · 109 comments
Closed

Integrated Terminal Lags Intermittently #105446

davidzech opened this issue Aug 26, 2020 · 109 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders macos-big-sur perf terminal Integrated terminal issues upstream-issue-linked This is an upstream issue that has been reported upstream verified Verification succeeded
Milestone

Comments

@davidzech
Copy link

davidzech commented Aug 26, 2020

  • VSCode Version: 1.48.2(macOS Stable)
  • OS Version: macOS Big Sur 11.0 Beta (20A5354i) (Public Beta Release 2)

Steps to Reproduce:

  1. Open Terminal
  2. Spam random keys until lag is experienced Press enter, then continuously type (anything) for about 2 seconds.
  3. Experience a very long stutter

Does this issue occur when all extensions are disabled?: Yes

This occurs on a public beta build of macOS 11. I've confirmed the same issue on two different MacBook Pro's on the same OS version, and same VS Code version.

Attached is a performance profile. You can see the spike in the integrated terminal response time at around 23000ms - 24000ms

image

Profile-20200826T124154.json.zip

@davidzech
Copy link
Author

It seems like

exec('lsof -OPl -p ' + this._ptyProcess.pid + ' | grep cwd', (error, stdout, stderr) => {

is taking a long time.

@davidzech
Copy link
Author

Benchmarking this call, it takes on average 800ms. It doesn't make sense, but this seems to be a Node.js x macOS beta type of issue.

@festiveelephantseal
Copy link

I have been having this issue as well with the insiders build of vscode

@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug terminal Integrated terminal issues perf labels Sep 3, 2020
@Tyriar Tyriar added this to the September 2020 milestone Sep 3, 2020
@Tyriar
Copy link
Member

Tyriar commented Sep 3, 2020

@deepak1556 do you know why an exec call would be blocking the thread?

@deepak1556 deepak1556 self-assigned this Sep 3, 2020
@Tyriar Tyriar removed their assignment Sep 15, 2020
@davidzech
Copy link
Author

davidzech commented Sep 15, 2020

Some further notes for reference:

Monkey patching

to be

async() => { this._updateProcessCwd(); }

Seems to fix the terminal from being blocked. It's still a bandaid over the underlying problem, which is child_process.exec() taking a long time to execute on macOS Big Sur Beta.

Edit: I was wrong, still exhibits the behavior, weird.

@matinzd
Copy link

matinzd commented Oct 13, 2020

+1
I have this issue either and it happened after updating to macOS 11 (Big Sur) beta. I am currently on beta 9 and the issue still exists.

UPDATE:
I have the lag even if I am typing or running commands and its so annoying.

Version: 1.50.0
Commit: 93c2f0fbf16c5a4b10e4d5f89737d9c2c25488a3
Date: 2020-10-07T06:00:00.397Z (6 days ago)
Electron: 9.2.1
Chrome: 83.0.4103.122
Node.js: 12.14.1
V8: 8.3.110.13-electron.0
OS: Darwin x64 20.1.0

@tolgaerdonmez
Copy link

+1
I have this issue either and it happened after updating to macOS 11 (Big Sur) beta. I am currently on beta 9 and the issue still exists.

Version: 1.50.0
Commit: 93c2f0fbf16c5a4b10e4d5f89737d9c2c25488a3
Date: 2020-10-07T06:00:00.397Z (6 days ago)
Electron: 9.2.1
Chrome: 83.0.4103.122
Node.js: 12.14.1
V8: 8.3.110.13-electron.0
OS: Darwin x64 20.1.0

It also occurred with me after updating to Big Sur Beta, I think it is about the beta.

@davidzech
Copy link
Author

davidzech commented Oct 29, 2020

Some further insights:

I have done some debugging with lldb, and have narrowed it down to fork() being the bottleneck here.

Edit:
You can observe calls with
sudo dtruss -p <window_pid> -t fork -e

@Vardiak
Copy link

Vardiak commented Oct 31, 2020

@davidzech Don't you think this might be a bug from the macOS kernel then?

@Adam2Marsh
Copy link

+1 for having this issue; only remember seeing this issue after upgrading to Big Sur. Happy to run some debugging if anyone has any ideas.

Thanks.

@davidzech
Copy link
Author

davidzech commented Nov 4, 2020

@Adam2Marsh Have you built locally? To my surprise, I ran my tests on master and tag 1.50.1 and I do not experience this lag at all, yet is persistent in both Stable and Insider public releases.

An useful bisection perhaps.

See
image

Edit 2: VSCodium also doesn't exhibit this behavior for me.

@KangDroid
Copy link

@davidzech
+1 Agree, I built Code-OSS[master branch] with macOS 11.0.1[build = 20B5012d]
and I see no problem with input-lag with integrated terminal. Only for local build.
Public version of vscode still has that problem tho.

@deepak1556
Copy link
Collaborator

Thanks for narrowing down the issue @davidzech , we do use a custom electron build for the stable and insiders while we use https://electronjs.org/ releases for the OSS build. But I don't see any patches around the child_process code path.

Can you run the test on following electron builds, you can extract and run the electron binary, toggle developer tools from the View menu

@marcello3d
Copy link

FYI the root cause has already been tracked down and being fixed in libuv/electron, see:

@pdesantis
Copy link

To piggyback on what @marcello3d said, this was affecting our own Electron app, so we built a custom version of Electron using that libuv patch and it solved the issue for us. Hopefully this will be merged upstream soon, but in the meantime, other developers can try building Electron with that patch.

@deepak1556
Copy link
Collaborator

Thanks @marcello3d @pdesantis for the work on fix.

Just to update, whenever upstream releases a new version with the patch vscode will update to it. Also we didn't build the patch internally because it was too late in the current January iteration to get enough testing before stable release 1.53 that will happen next week. We will get this fix in to our internal builds next February iteration for early testing if there is no release from upstream.

I see that the issue is getting side tracked again, will lock until further updates.

@microsoft microsoft locked as too heated and limited conversation to collaborators Jan 27, 2021
@microsoft microsoft unlocked this conversation Feb 5, 2021
@deepak1556
Copy link
Collaborator

The libuv fix has been backported internally and is available in latest insiders, please verify if this issue is fixed. Thanks!

@deepak1556
Copy link
Collaborator

\closedWith ff85144

@deepak1556 deepak1556 added the author-verification-requested Issues potentially verifiable by issue author label Feb 5, 2021
@github-actions
Copy link

github-actions bot commented Feb 5, 2021

This bug has been fixed in to the latest release of VS Code Insiders!

@davidzech, you can help us out by confirming things are working as expected in the latest Insiders release. If things look good, please leave a comment with the text /verified to let us know. If not, please ensure you're on version 52838cf of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

@davidzech
Copy link
Author

/verified

Insiders Build 52838cf:
image

Current Stable Build 8490d3d:
image

@m1nkeh
Copy link

m1nkeh commented Feb 8, 2021

I got an update of the non-insiders today, and no issues that I could see either, weird.. v1.53

@darrenjennings
Copy link

oh thank God this was driving me crazy, thank you for fixing 🙏🏻

@ay0o
Copy link

ay0o commented Feb 10, 2021

so, what's the correct way of undoing codesign --remove-signature /Applications/Visual\ Studio\ Code.app/Contents/Frameworks/Code\ Helper\ \(Renderer\).app.

Guess we need to sign the app again. But with which key?

@MaciekBaron
Copy link

@ay0o the update should have installed a new signed binary

@superandrew
Copy link

is this distributed to everyone or just in a beta channel?

@gquittet
Copy link

I've received the update today and I can feel the improvement !
Thank you ! You rocks ! 💪

@leafac
Copy link

leafac commented Feb 13, 2021

I’m running version 1.53.2 and I still see a significant lag when opening new terminal panes. I ran the codesign command and it fixed the issue (I’ve been doing that for every new version…).

I think the issue hasn’t been solved for me. Am I doing something wrong?

@flash-gordon
Copy link
Contributor

@leafac the fix is available in the insiders builds only as of now. Switch to insiders builds or stick to the codesign workaround for the time being.

@leafac
Copy link

leafac commented Feb 13, 2021

@flash-gordon: Ah, I had misunderstood the previous comments and I thought the fix was generally available. Thanks for the clarification 👍

@calrsom
Copy link

calrsom commented Feb 18, 2021

Heads up, the fix was reverted upstream: libuv/libuv#3050 (comment)

I'm not sure what that means for VSCode or if this issue should be reopened? I guess maybe it depends on how Electron handles this.

@deepak1556
Copy link
Collaborator

FYI, the fix is reverted in electron but I have added workarounds for the failing test cases mentioned in the libuv repo while it is being investigated, so I have not reverted the fix internally for vscode.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders macos-big-sur perf terminal Integrated terminal issues upstream-issue-linked This is an upstream issue that has been reported upstream verified Verification succeeded
Projects
None yet
Development

No branches or pull requests