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

CPU usage even when idle (due to cursor rendering) #22900

Closed
joliss opened this Issue Mar 20, 2017 · 64 comments

Comments

Projects
None yet
@joliss

joliss commented Mar 20, 2017

  • VSCode Version: 1.10.2 (8076a19)
  • OS Version: macOS Sierra 10.12.3

VS Code uses 13% CPU when focused and idle, draining battery. This is likely due to the blinking cursor rendering. I think CPU usage when focused-and-idle could ideally be near-0%.

To reproduce (this works with an empty settings file and all plugins disabled):

  1. Close all VS Code windows.
  2. Open a new window (File -> New Window). It will show the Welcome page.
  3. Open a new tab with an empty untitled file (File -> New Tab). The cursor is blinking.
  4. You should see VS Code consuming a non-negligible amount of CPU -- 13% on my 13" MacBook Pro.
  5. Cmd+Tab into some other application. Now the cursor isn't visible anymore.
  6. You should see VS Code consuming virtually no CPU.

I recorded a Timeline in the Developer Tools, and a cursory look suggests that the CPU activity comes from rendering the blinking cursor every 500ms.

Other macOS applications like Chrome or TextEdit show a blinking cursor without consuming much CPU, so I think this could surely be optimized away.

@joliss

This comment has been minimized.

Show comment
Hide comment
@joliss

joliss Mar 20, 2017

A workaround for folks who are similarly obsessed about battery life: Disabling cursor blinking will make CPU usage drop to 0. Here's the setting:

  "editor.cursorBlinking": "solid"

joliss commented Mar 20, 2017

A workaround for folks who are similarly obsessed about battery life: Disabling cursor blinking will make CPU usage drop to 0. Here's the setting:

  "editor.cursorBlinking": "solid"
@joliss

This comment has been minimized.

Show comment
Hide comment
@joliss

joliss Mar 21, 2017

Some people on Twitter said they couldn't reproduce this, so I went and recorded a timeline in the Developer Tools to help debug.

TimelineRawData-20170321T114212.json.zip

  • Screenshot of the timeline:

    boot mz0y1
  • Zooming into a single frame, we see that while we render only 2 fps, the main thread is performing some work at 60 fps (every 16 ms) -- the thin lines marked with arrows:

    boot 684m3

  • Zooming all the way into one of those thin lines, we see that some rendering is happening at 60 fps:

    boot f9qau
  • When I take a CPU profile, it shows most CPU being spent in "(program)", not any specific JS function.

    boot g2wbo
  • When I set "editor.cursorBlinking": "solid", the problem mostly goes away: A periodic "Update Layer Tree" / "Paint" / "Composite Layers" is still happening, but only every 500ms, not every 16ms.

I hope this helps!

joliss commented Mar 21, 2017

Some people on Twitter said they couldn't reproduce this, so I went and recorded a timeline in the Developer Tools to help debug.

TimelineRawData-20170321T114212.json.zip

  • Screenshot of the timeline:

    boot mz0y1
  • Zooming into a single frame, we see that while we render only 2 fps, the main thread is performing some work at 60 fps (every 16 ms) -- the thin lines marked with arrows:

    boot 684m3

  • Zooming all the way into one of those thin lines, we see that some rendering is happening at 60 fps:

    boot f9qau
  • When I take a CPU profile, it shows most CPU being spent in "(program)", not any specific JS function.

    boot g2wbo
  • When I set "editor.cursorBlinking": "solid", the problem mostly goes away: A periodic "Update Layer Tree" / "Paint" / "Composite Layers" is still happening, but only every 500ms, not every 16ms.

I hope this helps!

@rebornix

This comment has been minimized.

Show comment
Hide comment
@rebornix

rebornix Mar 22, 2017

Member

@joliss Quick answer to what's happening under the hood: a native animation updates at 60hz in Chromium.

Similar (almost the same) Chromium issue https://bugs.chromium.org/p/chromium/issues/detail?id=500259 and Chromium's tracking item https://bugs.chromium.org/p/chromium/issues/detail?id=361587

Our current CSS animation

@keyframes monaco-cursor-blink {
	50% {
		opacity: 0;
	}
	100% {
		opacity: 1;
	}
}

.cursor-blink {
	animation: monaco-cursor-blink 1s step-start 0s infinite;
}

Update

Quoting Paul Irish (Chrome guy) https://news.ycombinator.com/item?id=13941293

Powerful* text editors built on the web stack cannot rely on the OS text caret and have to provide their own.
In this case, VSCode is probably using the most reasonable approach to blinking a cursor: a step timing-function with a CSS keyframe animation. This tells the browser to only change the opacity every 500ms. Meanwhile, Chrome hasn't yet optimised this completely yet, hence http://crbug.com/361587.
So currently, Chrome is doing the full rendering lifecycle (style, paint, layers) every 16ms when it should be only doing that work at a 500ms interval. I'm confident that the engineers working on Chrome's style components can sort this out, but it'll take a little bit of work. I think the added visibility on this topic will likely escalate the priority of the fix. :)

  • Simple text editors, and basic ones built on [contenteditable] can, but those rarely scale to the feature set most want.
Member

rebornix commented Mar 22, 2017

@joliss Quick answer to what's happening under the hood: a native animation updates at 60hz in Chromium.

Similar (almost the same) Chromium issue https://bugs.chromium.org/p/chromium/issues/detail?id=500259 and Chromium's tracking item https://bugs.chromium.org/p/chromium/issues/detail?id=361587

Our current CSS animation

@keyframes monaco-cursor-blink {
	50% {
		opacity: 0;
	}
	100% {
		opacity: 1;
	}
}

.cursor-blink {
	animation: monaco-cursor-blink 1s step-start 0s infinite;
}

Update

Quoting Paul Irish (Chrome guy) https://news.ycombinator.com/item?id=13941293

Powerful* text editors built on the web stack cannot rely on the OS text caret and have to provide their own.
In this case, VSCode is probably using the most reasonable approach to blinking a cursor: a step timing-function with a CSS keyframe animation. This tells the browser to only change the opacity every 500ms. Meanwhile, Chrome hasn't yet optimised this completely yet, hence http://crbug.com/361587.
So currently, Chrome is doing the full rendering lifecycle (style, paint, layers) every 16ms when it should be only doing that work at a 500ms interval. I'm confident that the engineers working on Chrome's style components can sort this out, but it'll take a little bit of work. I think the added visibility on this topic will likely escalate the priority of the fix. :)

  • Simple text editors, and basic ones built on [contenteditable] can, but those rarely scale to the feature set most want.
@graymalkin

This comment has been minimized.

Show comment
Hide comment
@graymalkin

graymalkin Mar 23, 2017

Will this change about background CPU usage of chromium tabs affect the editor?

Will this change about background CPU usage of chromium tabs affect the editor?

@bpasero

This comment has been minimized.

Show comment
Hide comment
@bpasero

bpasero Mar 23, 2017

Member

Hopefully not (see electron/electron#7553). We do set backgroundThrottling to false since a while already.

Member

bpasero commented Mar 23, 2017

Hopefully not (see electron/electron#7553). We do set backgroundThrottling to false since a while already.

@alexandrudima

This comment has been minimized.

Show comment
Hide comment
@alexandrudima

alexandrudima Mar 23, 2017

Member

Thank you @joliss @rebornix for the nice analysis.

Looks like we should go back to using setInterval on OSX.

p.s. Here's the initial cursor blinking logic :)
image

Member

alexandrudima commented Mar 23, 2017

Thank you @joliss @rebornix for the nice analysis.

Looks like we should go back to using setInterval on OSX.

p.s. Here's the initial cursor blinking logic :)
image

@bcherny

This comment has been minimized.

Show comment
Hide comment
@bcherny

bcherny Mar 23, 2017

@rebornix Here's a similar issue we ran into a while ago - https://bugs.chromium.org/p/chromium/issues/detail?id=658894. The workaround in our case was to remove the animating element from the DOM when it was not used, rather than just occluding it.

bcherny commented Mar 23, 2017

@rebornix Here's a similar issue we ran into a while ago - https://bugs.chromium.org/p/chromium/issues/detail?id=658894. The workaround in our case was to remove the animating element from the DOM when it was not used, rather than just occluding it.

@alexandrudima alexandrudima added the bug label Mar 23, 2017

@alexandrudima alexandrudima added this to the March 2017 milestone Mar 23, 2017

@alexandrudima alexandrudima added the perf label Mar 23, 2017

@sunnykgupta

This comment has been minimized.

Show comment
Hide comment
@sunnykgupta

sunnykgupta Mar 23, 2017

There is also a css style for the same, not sure if it is used here though -

@keyframes monaco-cursor-blink {
	50% {
		opacity: 0;
	}
	100% {
		opacity: 1;
	}
}

https://github.com/Microsoft/vscode/blob/master/src/vs/editor/browser/viewParts/viewCursors/viewCursors.css

There is also a css style for the same, not sure if it is used here though -

@keyframes monaco-cursor-blink {
	50% {
		opacity: 0;
	}
	100% {
		opacity: 1;
	}
}

https://github.com/Microsoft/vscode/blob/master/src/vs/editor/browser/viewParts/viewCursors/viewCursors.css

@camwest

This comment has been minimized.

Show comment
Hide comment
@jlukic

This comment has been minimized.

Show comment
Hide comment
@jlukic

jlukic Mar 23, 2017

Use pageVisibility API's which let you know when page is hidden to disable the animation.

function handleVisibilityChange() {
  if (document.hidden) {
    // disable cursor animation with class name
  } 
  else  {
    // add back cursor animation
  }
}
document.addEventListener("visibilitychange", handleVisibilityChange, false);

jlukic commented Mar 23, 2017

Use pageVisibility API's which let you know when page is hidden to disable the animation.

function handleVisibilityChange() {
  if (document.hidden) {
    // disable cursor animation with class name
  } 
  else  {
    // add back cursor animation
  }
}
document.addEventListener("visibilitychange", handleVisibilityChange, false);
@mehas

This comment has been minimized.

Show comment
Hide comment
@mehas

mehas Mar 23, 2017

Suggestions here to render the CSS animation using the GPU instead of the CPU:

.cursor-blink {
	will-change: opacity;
}

mehas commented Mar 23, 2017

Suggestions here to render the CSS animation using the GPU instead of the CPU:

.cursor-blink {
	will-change: opacity;
}
@rebornix

This comment has been minimized.

Show comment
Hide comment
@rebornix

rebornix Mar 23, 2017

Member

In current implementation, if the editor loses focus, we'll remove the animation so life is easy. However if the editor has focus, it means the editor is visible (not hidden or in background) and active, users are working on it (reading is a good case) but don't trigger any view/content change. In this case we still need to display the blinking cursor even though it's kind of idle, that's what a blinking cursor is.

Initially this feature is implemented in JavaScript and about one year ago, we switched to CSS animation. Like @alexandrudima mentioned, we may want to move back to JS on OSX.

@camwest that api is charming, the only catch is compatibility (sadly Safari).

@mehas will-change is promising. I gave it a try, but Chromium doesn't optimize in this case. If you turn on Paint Flashing option in Dev Tools, you can see that this blinking cursor is not being repainted at all.

@jlukic @bcherny thanks for your suggestions. The thing we want to optimize is when the cursor is visible, active and blinking, so we still have this issue even though we have visibility check. But you are right, we should check visibility. Right now if you scroll the window to make the blinking cursor invisible, we don't have any optimization.

Member

rebornix commented Mar 23, 2017

In current implementation, if the editor loses focus, we'll remove the animation so life is easy. However if the editor has focus, it means the editor is visible (not hidden or in background) and active, users are working on it (reading is a good case) but don't trigger any view/content change. In this case we still need to display the blinking cursor even though it's kind of idle, that's what a blinking cursor is.

Initially this feature is implemented in JavaScript and about one year ago, we switched to CSS animation. Like @alexandrudima mentioned, we may want to move back to JS on OSX.

@camwest that api is charming, the only catch is compatibility (sadly Safari).

@mehas will-change is promising. I gave it a try, but Chromium doesn't optimize in this case. If you turn on Paint Flashing option in Dev Tools, you can see that this blinking cursor is not being repainted at all.

@jlukic @bcherny thanks for your suggestions. The thing we want to optimize is when the cursor is visible, active and blinking, so we still have this issue even though we have visibility check. But you are right, we should check visibility. Right now if you scroll the window to make the blinking cursor invisible, we don't have any optimization.

@joelday

This comment has been minimized.

Show comment
Hide comment
@joelday

joelday Mar 23, 2017

Contributor

To be fair, Monaco is really, really, really complex. :)

Contributor

joelday commented Mar 23, 2017

To be fair, Monaco is really, really, really complex. :)

@mrkite

This comment has been minimized.

Show comment
Hide comment
@mrkite

mrkite Mar 23, 2017

@rebornix will-change worked for my project, cpu usage disappeared entirely. However, my keyframes don't change opacity.. instead they change the element's color from 'inherit' to 'transparent'.

mrkite commented Mar 23, 2017

@rebornix will-change worked for my project, cpu usage disappeared entirely. However, my keyframes don't change opacity.. instead they change the element's color from 'inherit' to 'transparent'.

@eteeselink

This comment has been minimized.

Show comment
Hide comment
@eteeselink

eteeselink Mar 23, 2017

I'm just a lurker, and sorry if this is considered swearing, but wouldn't a two-frame animated gif do the trick? i'm not entirely sure how to test this conclusively, but I could imagine that even KHTML already had optimized paths for that, long before it transgromorphed into Chrome.

I'm just a lurker, and sorry if this is considered swearing, but wouldn't a two-frame animated gif do the trick? i'm not entirely sure how to test this conclusively, but I could imagine that even KHTML already had optimized paths for that, long before it transgromorphed into Chrome.

@matthiasg

This comment has been minimized.

Show comment
Hide comment
@matthiasg

matthiasg Mar 23, 2017

@eteeselink would be interesting to see, but cursor color needs to change and zoom etc. might not be trivial, switching back to js in the meantime might be easier.

@eteeselink would be interesting to see, but cursor color needs to change and zoom etc. might not be trivial, switching back to js in the meantime might be easier.

@eteeselink

This comment has been minimized.

Show comment
Hide comment
@eteeselink

eteeselink Mar 23, 2017

@matthiasg Ahyes, forgot about zoom.

Otherwise, autogenerating a blinking GIF based on the theme color shouldn't be too hard. It's probably a matter of patching a few bytes in a pre-baked file because while GIF pixel data is LZW-encoded, the palette data is uncompressed. But I bet zooming a gif like that makes it blurry, so maybe this is bad approach nonetheless.

If someone can come up with a way to make zooming a gif caret not suck, then I promise to contribute code that takes a color and produces a blinking-caret animated gif data url.

eteeselink commented Mar 23, 2017

@matthiasg Ahyes, forgot about zoom.

Otherwise, autogenerating a blinking GIF based on the theme color shouldn't be too hard. It's probably a matter of patching a few bytes in a pre-baked file because while GIF pixel data is LZW-encoded, the palette data is uncompressed. But I bet zooming a gif like that makes it blurry, so maybe this is bad approach nonetheless.

If someone can come up with a way to make zooming a gif caret not suck, then I promise to contribute code that takes a color and produces a blinking-caret animated gif data url.

@Ultrabenosaurus

This comment has been minimized.

Show comment
Hide comment
@Ultrabenosaurus

Ultrabenosaurus Mar 23, 2017

@eteeselink you should absolutely gist that in vanilla JS anyway, maybe PHP and a few other flavours as well. I can imagine it would get several tonnes of usage all over the web, and then you'll be super-famous if you used a license that requires acknowledging the original author.

@eteeselink you should absolutely gist that in vanilla JS anyway, maybe PHP and a few other flavours as well. I can imagine it would get several tonnes of usage all over the web, and then you'll be super-famous if you used a license that requires acknowledging the original author.

@julianlam

This comment has been minimized.

Show comment
Hide comment
@julianlam

julianlam Mar 23, 2017

@eteeselink If we're going around suggesting off-the-wall solutions, why don't we wrap the carat in <blink></blink> instead? 😉

@eteeselink If we're going around suggesting off-the-wall solutions, why don't we wrap the carat in <blink></blink> instead? 😉

rebornix added a commit to rebornix/vscode that referenced this issue Mar 23, 2017

@joshfriend

This comment has been minimized.

Show comment
Hide comment
@joshfriend

joshfriend Mar 23, 2017

Please refrain from joining the Reddit brigade in making useless comments on issues and PRs where people are trying to make real improvements.

joshfriend commented Mar 23, 2017

Please refrain from joining the Reddit brigade in making useless comments on issues and PRs where people are trying to make real improvements.

@o11c

This comment has been minimized.

Show comment
Hide comment
@o11c

o11c Mar 23, 2017

if the editor has focus, it means the editor is visible

This is a false assumption. It's trivial to raise a window over another without giving it focus. Two ways I know of:

  • the "always draw on top" WM hint
  • when a window is moved between virtual desktops (since the stacking order is global, if a window was foremost on the previous desktop, it might not be on the new one).

Both are very common on their own, though they don't necessarily always cause problems.

o11c commented Mar 23, 2017

if the editor has focus, it means the editor is visible

This is a false assumption. It's trivial to raise a window over another without giving it focus. Two ways I know of:

  • the "always draw on top" WM hint
  • when a window is moved between virtual desktops (since the stacking order is global, if a window was foremost on the previous desktop, it might not be on the new one).

Both are very common on their own, though they don't necessarily always cause problems.

@rebornix

This comment has been minimized.

Show comment
Hide comment
@rebornix

rebornix Mar 23, 2017

Member

@o11c thanks, my assumption was too wild when writing. Yes focus doesn't necessary means visible, scrolling the window is one case (I mentioned this in the same paragraph :( ). I don't know if focus+visible is the most common case but all of them should be mitigated.

Member

rebornix commented Mar 23, 2017

@o11c thanks, my assumption was too wild when writing. Yes focus doesn't necessary means visible, scrolling the window is one case (I mentioned this in the same paragraph :( ). I don't know if focus+visible is the most common case but all of them should be mitigated.

@esprehn

This comment has been minimized.

Show comment
Hide comment
@esprehn

esprehn Mar 23, 2017

I think setTimeout or setInterval is probably a better fit for this use case than requestIdleCallback. requestIdleCallback's timing is not predictable, and the work you're doing in JS is inexpensive, I think you'd be better off just scheduling infrequent timers.

ex. https://gist.github.com/esprehn/afec30fbc655bba6bb8f3f67c28beef4

Also of note is that this caret animation is doing a smooth pulse effect, but the system cursor in browsers (ex. the one inside an <input> or <textarea>) is only doing a binary on/off. Native controls on Mac and Linux similarly do a blink instead. It's less pretty, but would use considerably less power.

esprehn commented Mar 23, 2017

I think setTimeout or setInterval is probably a better fit for this use case than requestIdleCallback. requestIdleCallback's timing is not predictable, and the work you're doing in JS is inexpensive, I think you'd be better off just scheduling infrequent timers.

ex. https://gist.github.com/esprehn/afec30fbc655bba6bb8f3f67c28beef4

Also of note is that this caret animation is doing a smooth pulse effect, but the system cursor in browsers (ex. the one inside an <input> or <textarea>) is only doing a binary on/off. Native controls on Mac and Linux similarly do a blink instead. It's less pretty, but would use considerably less power.

@rebornix

This comment has been minimized.

Show comment
Hide comment
@rebornix

rebornix Mar 24, 2017

Member

Thanks @esprehn , that's exactly what I did for flat blinking right now. And like you mentioned, it's not as pretty as animation, not even to say smooth/expand blinking cursors which use ease-in-out.

Member

rebornix commented Mar 24, 2017

Thanks @esprehn , that's exactly what I did for flat blinking right now. And like you mentioned, it's not as pretty as animation, not even to say smooth/expand blinking cursors which use ease-in-out.

@mrkev

This comment has been minimized.

Show comment
Hide comment
@mrkev

mrkev Mar 24, 2017

wait @eteeselink, wouldn't you want to use a 1px gif and resize it anyway? Blur shouldn't be a problem.

Example: https://jsfiddle.net/mrkev/stxq613s/1/

Hoping to see that animated blinking-caret generator soon ;)

mrkev commented Mar 24, 2017

wait @eteeselink, wouldn't you want to use a 1px gif and resize it anyway? Blur shouldn't be a problem.

Example: https://jsfiddle.net/mrkev/stxq613s/1/

Hoping to see that animated blinking-caret generator soon ;)

@rmacfadyen

This comment has been minimized.

Show comment
Hide comment
@rmacfadyen

rmacfadyen Mar 24, 2017

@mrkev your 1px gif as data uri: data:image/gif;base64,R0lGODlhAQABAPAAAAAAAP///yH/C05FVFNDQVBFMi4wAwEAAAAh+QQFMgABACwAAAAAAQABAAACAkwBACH5BAUyAAEALAAAAAABAAEAAAICRAEAOw==

@mrkev your 1px gif as data uri: data:image/gif;base64,R0lGODlhAQABAPAAAAAAAP///yH/C05FVFNDQVBFMi4wAwEAAAAh+QQFMgABACwAAAAAAQABAAACAkwBACH5BAUyAAEALAAAAAABAAEAAAICRAEAOw==

@mrkev

This comment has been minimized.

Show comment
Hide comment
@mrkev

mrkev Mar 24, 2017

@rmacfadyen sweet! since gifs use a color table in theory the 3 bytes that color that pixel black on the "on" frame shouldn't be too hard to find (they are not stored in the frame, so probably no need to go too deep past the header). If I find some spare time tomorrow I can dive deeper into it too

mrkev commented Mar 24, 2017

@rmacfadyen sweet! since gifs use a color table in theory the 3 bytes that color that pixel black on the "on" frame shouldn't be too hard to find (they are not stored in the frame, so probably no need to go too deep past the header). If I find some spare time tomorrow I can dive deeper into it too

@mrkev

This comment has been minimized.

Show comment
Hide comment
@mrkev

mrkev Mar 24, 2017

Welp, who needs sleep anyway, decided to explore this a bit further. Since gifs use a global color table that always starts on the 14th byte it wasn't hard to figure out how to change the color. This made this blinking pixel gif red:

screen shot 3

Now what's really annoying is that in base64 each digit encodes only 6 bits so things don't align properly. It's the least significant bits of digit 18 up to the most significant of digit 22 encode the color black in that gif. So essentially some permutation of characters [A-P] [A-/] [A-/] [A-/] [P,f,v,/] in position 18-22 will draw that pixel in every color.

Here's an example of that GIF on some random color. I essentially just replaced whatever was in characters 18-22 with G8ABf (which fits the criteria I speak of above).

https://jsfiddle.net/mrkev/stxq613s/7/

Shouldn't be too bad to build a function that takes RGB and returns this gif in that color (:

mrkev commented Mar 24, 2017

Welp, who needs sleep anyway, decided to explore this a bit further. Since gifs use a global color table that always starts on the 14th byte it wasn't hard to figure out how to change the color. This made this blinking pixel gif red:

screen shot 3

Now what's really annoying is that in base64 each digit encodes only 6 bits so things don't align properly. It's the least significant bits of digit 18 up to the most significant of digit 22 encode the color black in that gif. So essentially some permutation of characters [A-P] [A-/] [A-/] [A-/] [P,f,v,/] in position 18-22 will draw that pixel in every color.

Here's an example of that GIF on some random color. I essentially just replaced whatever was in characters 18-22 with G8ABf (which fits the criteria I speak of above).

https://jsfiddle.net/mrkev/stxq613s/7/

Shouldn't be too bad to build a function that takes RGB and returns this gif in that color (:

@mrkev

This comment has been minimized.

Show comment
Hide comment
@mrkev

mrkev Mar 24, 2017

I have class in 7 hours I'm literally just playing myself rn.

But whatever, I realized someone probably wrote a hex-to-base64 function already and a quick stackoverflow search was enough to find it. So here's the function;

// takes color as string 'RRGGBB'
var generate_cursor_image = color => {
  var gif = '47494638396101000100F00000' + color + '00000021FF0B4E45545343415045322E30030100000021F90405320001002C00000000010001000002024C010021F90405320001002C00000000010001000002024401003B'
  return 'data:image/gif;base64,' + btoa(gif.match(/\w{2}/g).map(a => String.fromCharCode(parseInt(a, 16))).join(""))
}

Good night everyone!

mrkev commented Mar 24, 2017

I have class in 7 hours I'm literally just playing myself rn.

But whatever, I realized someone probably wrote a hex-to-base64 function already and a quick stackoverflow search was enough to find it. So here's the function;

// takes color as string 'RRGGBB'
var generate_cursor_image = color => {
  var gif = '47494638396101000100F00000' + color + '00000021FF0B4E45545343415045322E30030100000021F90405320001002C00000000010001000002024C010021F90405320001002C00000000010001000002024401003B'
  return 'data:image/gif;base64,' + btoa(gif.match(/\w{2}/g).map(a => String.fromCharCode(parseInt(a, 16))).join(""))
}

Good night everyone!

@eteeselink

This comment has been minimized.

Show comment
Hide comment
@eteeselink

eteeselink Mar 24, 2017

Damn, @mrkev beat me to it. Anyway, here's my implementation of the same thing:
https://jsfiddle.net/a6g4ob7h/

It might be a bit faster because there's no matching and mapping going on, just a btoa. But I doubt speed matters, and it's better to cache the result.

Damn, @mrkev beat me to it. Anyway, here's my implementation of the same thing:
https://jsfiddle.net/a6g4ob7h/

It might be a bit faster because there's no matching and mapping going on, just a btoa. But I doubt speed matters, and it's better to cache the result.

@eteeselink

This comment has been minimized.

Show comment
Hide comment
@eteeselink

eteeselink Mar 24, 2017

The thing I really wonder is whether this speeds things up at all. I don't have a good setup to test, but who knows, maybe animated gifs are rerendered at 60fps as well.

The thing I really wonder is whether this speeds things up at all. I don't have a good setup to test, but who knows, maybe animated gifs are rerendered at 60fps as well.

@rkeytacked

This comment has been minimized.

Show comment
Hide comment
@rkeytacked

rkeytacked Mar 24, 2017

If you want to zoom and not have rectangular-shaped cursor you could also go with animated SVG. (The <animate> element was already part of the SVG 1.0 spec.)

rkeytacked commented Mar 24, 2017

If you want to zoom and not have rectangular-shaped cursor you could also go with animated SVG. (The <animate> element was already part of the SVG 1.0 spec.)

alexandrudima added a commit that referenced this issue Mar 24, 2017

Merge pull request #23121 from rebornix/BlinkingCursor
Mitigate #22900. Use setInterval instead of animation for flat blinking on mac/linux
@danechitoaie

This comment has been minimized.

Show comment
Hide comment
@danechitoaie

danechitoaie Mar 24, 2017

out

For me it stays to 0 most of the time (cpu usage is the first column - OSX 10.12.3 - MacBook Pro Retina, 15-inch, Late 2013)

I have default settings in regards to cursor.

out

For me it stays to 0 most of the time (cpu usage is the first column - OSX 10.12.3 - MacBook Pro Retina, 15-inch, Late 2013)

I have default settings in regards to cursor.

@lnicola

This comment has been minimized.

Show comment
Hide comment
@lnicola

lnicola Mar 24, 2017

As nobody mentioned Linux, I have it at around 5-7% on GNOME Shell with Wayland (Ivy Bridge Graphics).

lnicola commented Mar 24, 2017

As nobody mentioned Linux, I have it at around 5-7% on GNOME Shell with Wayland (Ivy Bridge Graphics).

@alexandrudima

This comment has been minimized.

Show comment
Hide comment
@alexandrudima

alexandrudima Mar 24, 2017

Member

We've just merged PR #23121 that mitigates this by going back to setInterval for the blink cursor blinking style. On the mac mini I have at my disposal the CPU usage drops from 5.9% to 0.9%

Member

alexandrudima commented Mar 24, 2017

We've just merged PR #23121 that mitigates this by going back to setInterval for the blink cursor blinking style. On the mac mini I have at my disposal the CPU usage drops from 5.9% to 0.9%

@justjoeyuk

This comment has been minimized.

Show comment
Hide comment
@justjoeyuk

justjoeyuk Mar 24, 2017

Have you ever considered going native and having the OS provide the cursor, free of charge?

Have you ever considered going native and having the OS provide the cursor, free of charge?

@julianlam

This comment has been minimized.

Show comment
Hide comment
@joelday

This comment has been minimized.

Show comment
Hide comment
@joelday

joelday Mar 24, 2017

Contributor

@justjoeyuk If instead you are suggesting a fully native, non web stack based application: when you traverse the likely needs of a heavily themed, fully DPI independent, cross-platform application like VSCode, odds are that a non-native cursor substitute would be needed regardless.

Contributor

joelday commented Mar 24, 2017

@justjoeyuk If instead you are suggesting a fully native, non web stack based application: when you traverse the likely needs of a heavily themed, fully DPI independent, cross-platform application like VSCode, odds are that a non-native cursor substitute would be needed regardless.

@LandonPowell

This comment has been minimized.

Show comment
Hide comment
@LandonPowell

LandonPowell Mar 24, 2017

BREAKING NEWS! Microsoft makes horribly coded slow software, with the issues being even more prominent on MacOS 10.

Who would have ever guessed, right?

LandonPowell commented Mar 24, 2017

BREAKING NEWS! Microsoft makes horribly coded slow software, with the issues being even more prominent on MacOS 10.

Who would have ever guessed, right?

@phansen01

This comment has been minimized.

Show comment
Hide comment
@phansen01

phansen01 Mar 24, 2017

@LandonPowell You're polluting actual discussion about an issue; the reddit/hackernews brigading here is ridiculous.

@LandonPowell You're polluting actual discussion about an issue; the reddit/hackernews brigading here is ridiculous.

@zelein

This comment has been minimized.

Show comment
Hide comment
@zelein

zelein Mar 24, 2017

What about the terminal cursor? Is that causing spikes in CPU cycles as well?

zelein commented Mar 24, 2017

What about the terminal cursor? Is that causing spikes in CPU cycles as well?

@Daniel15

This comment has been minimized.

Show comment
Hide comment
@Daniel15

Daniel15 Mar 24, 2017

@mehas

Suggestions here to render the CSS animation using the GPU instead of the CPU:

That just hides the issue by moving the load from the CPU to the GPU. It doesn't actually fix it.

@mehas

Suggestions here to render the CSS animation using the GPU instead of the CPU:

That just hides the issue by moving the load from the CPU to the GPU. It doesn't actually fix it.

@mehas

This comment has been minimized.

Show comment
Hide comment
@mehas

mehas Mar 24, 2017

@Daniel15

That just hides the issue by moving the load from the CPU to the GPU. It doesn't actually fix it.

The load on the CPU is the problem, optimizing the animation to use the GPU does fix this Issue.

(That is separate from the question of the specific fix I suggested being the best one)

mehas commented Mar 24, 2017

@Daniel15

That just hides the issue by moving the load from the CPU to the GPU. It doesn't actually fix it.

The load on the CPU is the problem, optimizing the animation to use the GPU does fix this Issue.

(That is separate from the question of the specific fix I suggested being the best one)

@ken-experiment

This comment has been minimized.

Show comment
Hide comment
@ken-experiment

ken-experiment Mar 25, 2017

Not really sure how to reproduce it. I'm using the vim emulation plugin. Not really sure whether it's related.

I would like to know how this goes.

Not really sure how to reproduce it. I'm using the vim emulation plugin. Not really sure whether it's related.

I would like to know how this goes.

@Daniel15

This comment has been minimized.

Show comment
Hide comment
@Daniel15

Daniel15 Mar 25, 2017

optimizing the animation to use the GPU does fix this issue.

It'll result in unnecessary GPU load though. That's a similar issue, except the load is on the graphics card's processor. It's not really a fix 😛

optimizing the animation to use the GPU does fix this issue.

It'll result in unnecessary GPU load though. That's a similar issue, except the load is on the graphics card's processor. It's not really a fix 😛

@kNmnPyGGCwxk4zQoDD

This comment has been minimized.

Show comment
Hide comment
@kNmnPyGGCwxk4zQoDD

kNmnPyGGCwxk4zQoDD Mar 25, 2017

Jesus was born in Africa.

You're welcome.

Jesus was born in Africa.

You're welcome.

@efroim102

This comment has been minimized.

Show comment
Hide comment
@efroim102

efroim102 Mar 26, 2017

Why not use a gif for the cursor?

Why not use a gif for the cursor?

@mehas

This comment has been minimized.

Show comment
Hide comment
@mehas

mehas Mar 26, 2017

It'll result in unnecessary GPU load though. That's a similar issue

Right, but it's not this Issue, which is CPU usage even when idle.

@efroim102 #22900 (comment)

mehas commented Mar 26, 2017

It'll result in unnecessary GPU load though. That's a similar issue

Right, but it's not this Issue, which is CPU usage even when idle.

@efroim102 #22900 (comment)

@MikeTheCanuck

This comment has been minimized.

Show comment
Hide comment
@MikeTheCanuck

MikeTheCanuck Mar 26, 2017

Oh dear gods, this debate will go in circles until it is decided what is the goal:

  1. Reduce battery consumption
  2. Reduce CPU contention

Assume (1) for the moment. Then we can be a little more data-driven about whether offloading from CPU to GPU with a workload such as this (a) uses the same amount of energy, (b) uses less energy, or (c) uses more energy. I have no expertise in these matters so I cannot predict which is true.

However, let's assume that GPU offload leads to (b); while not perfect, the trade-off may be acceptable enough to pursue this path while/if Chromium address their underlying issue. However, if the VSCode team deem it necessary to fix this once and for all, then offload will be an unlikely choice, and pursuit of the long-term fix (even if it takes quite a while) is preferable. At least in my insignificant opinion, it'll be good enough for me to know that attention is on this problem and that it will be prioritized when dependencies enable the intended fix.

(I'm sure I've confused some of the subtle details of what solutions are available/blocked, but hoping in writing this that folks can focus on asserting what problem they're hoping to solve rather than leaping into solution space.)

Oh dear gods, this debate will go in circles until it is decided what is the goal:

  1. Reduce battery consumption
  2. Reduce CPU contention

Assume (1) for the moment. Then we can be a little more data-driven about whether offloading from CPU to GPU with a workload such as this (a) uses the same amount of energy, (b) uses less energy, or (c) uses more energy. I have no expertise in these matters so I cannot predict which is true.

However, let's assume that GPU offload leads to (b); while not perfect, the trade-off may be acceptable enough to pursue this path while/if Chromium address their underlying issue. However, if the VSCode team deem it necessary to fix this once and for all, then offload will be an unlikely choice, and pursuit of the long-term fix (even if it takes quite a while) is preferable. At least in my insignificant opinion, it'll be good enough for me to know that attention is on this problem and that it will be prioritized when dependencies enable the intended fix.

(I'm sure I've confused some of the subtle details of what solutions are available/blocked, but hoping in writing this that folks can focus on asserting what problem they're hoping to solve rather than leaping into solution space.)

@ZombieProtectionAgency

This comment has been minimized.

Show comment
Hide comment
@ZombieProtectionAgency

ZombieProtectionAgency Mar 27, 2017

Wouldn't the GPU be better equipped to deal with a rendering load either way? That is exactly what it was made for.

Wouldn't the GPU be better equipped to deal with a rendering load either way? That is exactly what it was made for.

@mehas

This comment has been minimized.

Show comment
Hide comment
@mehas

mehas Mar 27, 2017

Agreed, we prioritize what uses the CPU and GPU differently, having a graphics-related feature use up graphics-specific resources makes more sense. A gain of CPU is not a 1:1 loss of GPU, and vice-versa. Mostly moot at this point (until the bug in Chromium is patched) but somewhat relevant as current solution still employs the CPU (albeit with much less weight).

mehas commented Mar 27, 2017

Agreed, we prioritize what uses the CPU and GPU differently, having a graphics-related feature use up graphics-specific resources makes more sense. A gain of CPU is not a 1:1 loss of GPU, and vice-versa. Mostly moot at this point (until the bug in Chromium is patched) but somewhat relevant as current solution still employs the CPU (albeit with much less weight).

@bit2shift

This comment has been minimized.

Show comment
Hide comment
@bit2shift

bit2shift Mar 28, 2017

I think people should stop using web technology on the desktop, as it requires shipping a fully-fledged browser with each program.
Not to mention that, if you have several of said programs, you have the same binaries duplicated all over the place.

We need to go back to bare metal.

PS: JScript should've been ditched 10 years ago, just like Obj-C should've been, had Apple not resurrected it.
PPS: ^^^^ this is a rant.

I think people should stop using web technology on the desktop, as it requires shipping a fully-fledged browser with each program.
Not to mention that, if you have several of said programs, you have the same binaries duplicated all over the place.

We need to go back to bare metal.

PS: JScript should've been ditched 10 years ago, just like Obj-C should've been, had Apple not resurrected it.
PPS: ^^^^ this is a rant.

@eteeselink

This comment has been minimized.

Show comment
Hide comment
@eteeselink

eteeselink Mar 29, 2017

PPS: ^^^^ this is a rant.

@bit2shift Indeed it is, and as such it doesn't belong in the comment thread about a very specific bug. Please don't use GitHub issue comments for rants.

eteeselink commented Mar 29, 2017

PPS: ^^^^ this is a rant.

@bit2shift Indeed it is, and as such it doesn't belong in the comment thread about a very specific bug. Please don't use GitHub issue comments for rants.

@Daniel15

This comment has been minimized.

Show comment
Hide comment
@Daniel15

Daniel15 Mar 29, 2017

@bit2shift - While I agree with some parts (like the fact that a browser-based app probably won't feel quite "native" compared to a true native app), how low-level is "bare metal" though? Machine code? Assembly language? C? C++? C#? There's always going to be several layers of abstraction, regardless of your technology stack.

Not to mention that, if you have several of said programs, you have the same binaries duplicated all over the place.

It's the same with any C# app that uses NuGet packages though. All the assembles are local to the app. .NET Core even distributes large parts of the framework as NuGet packages.

as it requires shipping a fully-fledged browser with each program.

Technically you could use the Edge engine, which would avoid needing to install a separate browser runtime. Internet Explorer did something similar nearly 20 years ago with HTML Applications. Another example is React Native, which uses the OS' native JS engine where available. For JavaScript-based apps, something like React Native for Windows is probably the future rather than web technologies, since it feels more native as it uses native UI widgets.

@bit2shift - While I agree with some parts (like the fact that a browser-based app probably won't feel quite "native" compared to a true native app), how low-level is "bare metal" though? Machine code? Assembly language? C? C++? C#? There's always going to be several layers of abstraction, regardless of your technology stack.

Not to mention that, if you have several of said programs, you have the same binaries duplicated all over the place.

It's the same with any C# app that uses NuGet packages though. All the assembles are local to the app. .NET Core even distributes large parts of the framework as NuGet packages.

as it requires shipping a fully-fledged browser with each program.

Technically you could use the Edge engine, which would avoid needing to install a separate browser runtime. Internet Explorer did something similar nearly 20 years ago with HTML Applications. Another example is React Native, which uses the OS' native JS engine where available. For JavaScript-based apps, something like React Native for Windows is probably the future rather than web technologies, since it feels more native as it uses native UI widgets.

@bit2shift

This comment has been minimized.

Show comment
Hide comment
@bit2shift

bit2shift Mar 29, 2017

@Daniel15

(...) how low-level is "bare metal" though? Machine code? Assembly language? C? C++? C#?

Any language that is compilable to machine code (only source -> ELF/PE counts) is considered "bare metal" in this context, so layers of abstraction don't matter here.

Internet Explorer did something similar nearly 20 years ago with HTML Applications.

"something similar" as in having mshta.exe use mshtml.dll that resides in system32 since the '90s.

Another example is React Native, which uses the OS' native JS engine where available. For JavaScript-based apps, something like React Native for Windows is probably the future rather than web technologies, since it feels more native as it uses native UI widgets.

If high performance is desired in desktop JS apps, then someone with enough time in their hands should write a frontend for LLVM.
You'd lose the ability to run arbitrary snippets of code, but that would be an advantage from a security standpoint.

/thread

@Daniel15

(...) how low-level is "bare metal" though? Machine code? Assembly language? C? C++? C#?

Any language that is compilable to machine code (only source -> ELF/PE counts) is considered "bare metal" in this context, so layers of abstraction don't matter here.

Internet Explorer did something similar nearly 20 years ago with HTML Applications.

"something similar" as in having mshta.exe use mshtml.dll that resides in system32 since the '90s.

Another example is React Native, which uses the OS' native JS engine where available. For JavaScript-based apps, something like React Native for Windows is probably the future rather than web technologies, since it feels more native as it uses native UI widgets.

If high performance is desired in desktop JS apps, then someone with enough time in their hands should write a frontend for LLVM.
You'd lose the ability to run arbitrary snippets of code, but that would be an advantage from a security standpoint.

/thread

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Mar 29, 2017

Can you not derail this thread, please? This is an issue about a specific performance issue with a blinking cursor and it's definitely not a place for discussing general merits of developing JS-based desktop apps.

You're just spamming mailboxes of people that are not interested in these rants.

mgol commented Mar 29, 2017

Can you not derail this thread, please? This is an issue about a specific performance issue with a blinking cursor and it's definitely not a place for discussing general merits of developing JS-based desktop apps.

You're just spamming mailboxes of people that are not interested in these rants.

@sarim

This comment has been minimized.

Show comment
Hide comment
@sarim

sarim Mar 31, 2017

How about putting a <input type=text> with 2px * 10px size in place where the cursor it. That way it'll use the native blinking cursor of the <input> :P <blink></blink> 👍

Also i'm not sure how severe this issue is, when VSCode is idle (i'm not writing code) it is almost always not in focus, focus is on other application i'm in. And cursor stops blinking.

sarim commented Mar 31, 2017

How about putting a <input type=text> with 2px * 10px size in place where the cursor it. That way it'll use the native blinking cursor of the <input> :P <blink></blink> 👍

Also i'm not sure how severe this issue is, when VSCode is idle (i'm not writing code) it is almost always not in focus, focus is on other application i'm in. And cursor stops blinking.

@bboydflo

This comment has been minimized.

Show comment
Hide comment
@bboydflo

bboydflo Mar 31, 2017

from my experience, gifs are not so good. I once had a loading spinner as a gif image. and used it while interacting with indexeddb. sometimes I could see the spinner stop, even on the desktop.

from my experience, gifs are not so good. I once had a loading spinner as a gif image. and used it while interacting with indexeddb. sometimes I could see the spinner stop, even on the desktop.

@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Apr 4, 2017

Member

Pushed the terminal cursor change to master and release/1.11 (CSS animation -> setInterval).

Member

Tyriar commented Apr 4, 2017

Pushed the terminal cursor change to master and release/1.11 (CSS animation -> setInterval).

@rebornix

This comment has been minimized.

Show comment
Hide comment
@rebornix

rebornix Apr 4, 2017

Member

Thank you all for looking into this issue. Your investigations and suggestions helped us find the root cause and possible solutions! We compared JavaScript setInterval, animated gif and several other techniques, and we settled on the following solution:

  • If editor.cursorBlinking is set to blink or terminal.integrated.cursorBlinking is set to true, the blinking logic is now implemented in JavaScript. Our testing shows the CPU usage drops to less than 1%.
  • If editor.cursorBlinking is set to smooth, expand or phase, which use CSS easing functions, we'll stop the blinking animation after the cursor is idle for 10 seconds.

This fix is already in our Insider build and it will be available in our April Stable build which will be released in the next few days. It would be great if some of you could verify our test results. If you see problems, please create a new issue.

Member

rebornix commented Apr 4, 2017

Thank you all for looking into this issue. Your investigations and suggestions helped us find the root cause and possible solutions! We compared JavaScript setInterval, animated gif and several other techniques, and we settled on the following solution:

  • If editor.cursorBlinking is set to blink or terminal.integrated.cursorBlinking is set to true, the blinking logic is now implemented in JavaScript. Our testing shows the CPU usage drops to less than 1%.
  • If editor.cursorBlinking is set to smooth, expand or phase, which use CSS easing functions, we'll stop the blinking animation after the cursor is idle for 10 seconds.

This fix is already in our Insider build and it will be available in our April Stable build which will be released in the next few days. It would be great if some of you could verify our test results. If you see problems, please create a new issue.

@rebornix rebornix closed this Apr 4, 2017

@jedmao

This comment has been minimized.

Show comment
Hide comment
@jedmao

jedmao Apr 4, 2017

@rebornix did you mean to say "or" terminal.integrated.cursorBlinking is set to true? Or was the "and" intentional?

jedmao commented Apr 4, 2017

@rebornix did you mean to say "or" terminal.integrated.cursorBlinking is set to true? Or was the "and" intentional?

@rebornix

This comment has been minimized.

Show comment
Hide comment
@rebornix

rebornix Apr 5, 2017

Member

@jedmao thanks for the correction, it should or.

Member

rebornix commented Apr 5, 2017

@jedmao thanks for the correction, it should or.

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