3 ms·
Severe performance penalty found in VSCode rendering loop
- nawgz 11mo agoA bit sloppy but easily resolved - surprised it took so long to notice, or maybe it was new?
- minitech 11mo agoIt’s been around since the root commit in 2015: https://github.com/microsoft/vscode/blob/8f35cc4768393b25468416829e980d7550619fb1/src/vs/base/browser/dom.ts#L485-L486 https://github.com/microsoft/vscode/blob/8f35cc4768393b25468...
- anticensor 11mo agoYeah, it was a bit surprising to me as well.
- muglug 11mo agoGiven that the issue already gives a before-and-after metric it's extremely odd there's no POC PR attached. This just seems like an AI slop GitHub issue from beginning to end. And I'd be very surprised if VS Code performance could be boosted that much by a supposedly trivial fix.
- duskwuff 11mo agoEven if it is a real performance issue, the reasonable fix would be to move the sort call out of the loop - implementing a new data structure in JS is absolutely not the way to fix this.
- muglug 11mo agoRight, and also this would show up in the profiler if it were a time sink — and I'm 100% certain this code has been profiled in the 10 years it's been in the codebase.
- oe 11mo agoAdding a new data structure just for this feels like such an AI thing. I've added to our agents.md a rule to prefer using existing libraries and types, otherwise Gemini will just happily generate things like this.
- nneonneo 11mo agoThere’s clearly functionality to push more work to the current window’s queue, so I would not be surprised if the data structure needs to be continually kept sorted. (Somewhere in the pile of VSCode dependencies you’d think there’d be a generic heap data structure though)
- gigatexal 11mo agoI’ve already moved from VSCode to Zed. It’s native. Faster. Has most of the functionality I had before. I’m a huge fan.
- sillythrowawy9 11mo agoOP’s account also seems automated. This certainly feel like automated post to social media for PR clout
- anticensor 11mo agoNot really, I read HN more than I post to it, but I found this one interesting.
- ollin 11mo agoYeah the issue reads as if someone asked Claude Code "find the most serious performance issue in the VSCode rendering loop" and then copied the response directly into GitHub (without profiling or testing anything).
- a-dub 11mo agoi see emojis in the comments. also no discussion of measured runtimes for the rendering code. (if it saves ~1.3ms that sounds cool, but how many ms is that from going over the supposed 16ms budget.)
- hyperhello 11mo ago> Real-world impact: With 50+ view parts (text, cursors, minimap, scrollbar, widgets, decorations, etc.), this wastes 1-2ms per frame Good thing to find...
- blharr 11mo agoHow does it possibly take 1-2ms to sort... 50 items? I'd expect that to happen in an order of microseconds
- klodolph 11mo agoIt’s being sorted not once per frame, but once per item. If you have 50 items in the list, then the list gets sorted 50 times. If you have 200 items in the list, the list is sorted 200 times. This is unnecessary. The obvious alternative is a binary heap… which is what the fix does. Although it would also be obvious to reuse an existing binary heap implementation, rather than inventing your own.
- duskwuff 11mo ago> It’s being sorted not once per frame, but once per item. Even if that were the case, sorting a list that's already sorted is basically free. Any reasonable sort method (like the builtin one in a JS runtime) will check for that before doing anything to the list. > The obvious alternative is a binary heap… which is what the fix does. The overhead of creating a heap structure out of JS objects will dwarf any possible benefit of avoiding a couple of calls to Array.sort().
- minitech 11mo ago> Even if that were the case, sorting a list that's already sorted is basically free. Any reasonable sort method (like the builtin one in a JS runtime) will check for that before doing anything to the list. That’s n−1 pairs of elements that have to be compared with a a JS callback each time. (JavaScript’s combined language misfeatures that allow the array to be modified in many odd/indirect ways while it’s being sorted might add unusually high overhead to all sorting too – I’m not sure how much of a fast path there is.) Anyway, the code in question does include functionality to add new elements to the priority queue while it’s being processed. > The overhead of creating a heap structure out of JS objects will dwarf any possible benefit of avoiding a couple of calls to Array.sort(). Not true even in general and with an unoptimized heap implementation, and in this case, there’s an array of JS objects involved either way. In fact, there’s no number of elements small enough for sorting to be faster in this benchmark in my environment (I have no idea whether it reflects realistic conditions in VS Code, but it addresses the point): https://gist.github.com/minitech/7ff89dbf0c6394ce4861903a2325c43c https://gist.github.com/minitech/7ff89dbf0c6394ce4861903a232...
- webprofusion 11mo agoLooks interesting. I see they also contributed a fix to the OnlyFans notification robot. Clearly doing the important work that the internet needs.
- brokencode 11mo agoThis is what I want to do when I retire. Maybe not OnlyFans fixes specifically, but just go around fixing random stuff. Like if Batman turned out to be bad at fighting criminals so had to fight null pointer exceptions instead.
- gnarlouse 11mo agobugman
- OrderlyTiamat 11mo ago"Fear not the bugs citizen! For in my utility belt, I have REGEX and VIM!"
- nurettin 11mo agoMaybe hack into facilities, optimize their scripts and deployment, then leave without a trace confusing the IT department.
- anticensor 11mo agoThat notification robot codebase is actually generic, Zara Darcy just used OnlyFans branding to boost her follower base.
- adwn 11mo agoI'm confused: Does top.execute() modify currentQueue in some way, like pushing new elements to it? If it doesn't, then why not simply move the sort out of the loop? This is simpler and faster than maintaining a binary heap.
- adwn 11mo agoOne more thing: Nowadays sort() functions ary usually heavily optimized and recognize already sorted subsequences. If currentQueue isn't modified during the loop, then the sort() call should run in O(n) after the first iteration, instead of O(n * log n). Still worse than not having it inside the loop at all, of course.
- nateb2022 11mo ago> If it doesn't, then why not simply move the sort out of the loop? Yup, they should definitely move the sort outside of the loop. Shifting is O(N) so overall complexity would be O(N^2) but they could avoid shifting by reverse-sorting outside the loop and then iterating backwards using pop()
- jgoldshlag 11mo agoThis seems like a nonsense issue. Sorting 50 things takes 1-2 ms? What sort of potato was that timed on.
- tylerhou 11mo agoNo, sorting 50/2ish things 50 times allegedly takes 1-2ms. Which is only slightly more believable.
- jojobas 11mo agoThe penalty is called "Electron".
- geokon 11mo agoI feel with Valgrind (in C++land) or VisualVM (JVMland) stuff like this is very easy to zero in on. I don't work in JS-land.. but are Electron apps difficult to do performance profiling on?
- nateb2022 11mo agohttps://www.electronjs.org/docs/latest/tutorial/performance https://www.electronjs.org/docs/latest/tutorial/performance
- nawgz 11mo agoNo. Browser dev tools are available, and make it pretty easy to do performance profiling, and get a flamegraph etc.. Just seems like the reality of things is that the number of extensions or widgets or whatever has remained low enough that this extra sorting isn't actually that punitive in most real-world use cases. As a long-time developer working mainly in VSCode, I notice no difference between performance/snappiness in VSCode compared to JetBrains Rider, which is the main other IDE I have meaningful experience with these days.
- rockorager 11mo agoIf you work with LLM agents, you will immediately be able to tell this issue is written by one. The time cost of this sort is almost certainly not real, as others have pointed out. I’ve had agents find similar “performance bottlenecks” that are indeed BS.
- znpy 11mo agoas one of the commenters to the issue wrote, arguing whether the text is ai-generated or not is essentially useless. the important question is: is this an actual performance bug?
- cgriswald 11mo agoThe question is whether credence, and therefore time, should be given to the claim that the performance bug exists.
- ec109685 11mo agoI hate ai sometimes — an AI generated pull request (really some rando found a way of shaving 12% off the run loop?) responded to by an ai comment bot: > This feature request is now a candidate for our backlog. The community has 60 days to upvote the issue. If it receives 20 upvotes we will move it to our backlog. If not, we will close it. To learn more about how we handle feature requests, please see our documentation.
- flowerthoughts 11mo agoI would have expected V8 sort() to be optimized for runs of presorted input, like other implementations nowadays. So O(n²) seems more likely than O(n² log n). Not that it matters much. But then again, probably AI slop with "performance gain" numbers taken out of thin air. Who knows if the number 50 and 1-2ms are based on fantasy novels or not. Like when I used Claude to build a door video intercom sytem, and first asked it to create a plan. It inserted how many weeks each milestone would take, and it was an order of magnitude off. But I guess milestone documents have time estimates, so that's how it's supposed to look, information accuracy be damned.