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

massive slowdown from new scrollbar #5186

Open
jerch opened this issue Oct 6, 2024 · 7 comments
Open

massive slowdown from new scrollbar #5186

jerch opened this issue Oct 6, 2024 · 7 comments
Labels
area/performance type/bug Something is misbehaving type/debt Technical debt that could slow us down in the long run

Comments

@jerch
Copy link
Member

jerch commented Oct 6, 2024

Repro:

  • run ls -lR /usr in demo
  • move mouse off the terminal widget while the output flows

--> massive slowdown with high CPU usage, seems to be caused by tons of setTimeout/clearTimeout calls

image

@jerch jerch added type/bug Something is misbehaving type/debt Technical debt that could slow us down in the long run area/performance important labels Oct 6, 2024
@Tyriar
Copy link
Member

Tyriar commented Oct 7, 2024

@jerch timeouts are actually massively inflated in devtools so can't be trusted in a profile. This isn't actually a problem.

Back when we could use the JavaScript profiler tab in devtools this could be verified as all the overhead involved in the performance tab could be removed, but they removed this from Chromium unfortunately.

@Tyriar Tyriar closed this as completed Oct 7, 2024
@Tyriar
Copy link
Member

Tyriar commented Oct 7, 2024

Having said that, we could add some debouncing or something to the onDidScroll event which would make the profiles much less noisy. Ideally I think a single event per task would run a microtask.

@Tyriar Tyriar reopened this Oct 7, 2024
@Tyriar Tyriar removed the important label Oct 7, 2024
@jerch
Copy link
Member Author

jerch commented Oct 7, 2024

@jerch timeouts are actually massively inflated in devtools so can't be trusted in a profile. This isn't actually a problem.

Oh well, wasn't aware that setTimeout/clearTimeout are that toxic in the profiler. Its prolly the same with my other comment here --> #5176 (comment)

@jtbandes
Copy link
Contributor

I am indeed seeing tons of setTimeout/clearTimeout showing up in traces which supposedly contributes quite a bit to overall time spent in JS:

image

But it sounds like you're saying this may not really be true?

timeouts are actually massively inflated in devtools so can't be trusted in a profile

anywhere I can find some further reading or examples of this?

@jtbandes
Copy link
Contributor

I don't know if it means much since I don't use Firefox often, but the Firefox profiler also shows setTimeout at the top when I look at a bottom-up profile:

@jerch
Copy link
Member Author

jerch commented Dec 17, 2024

@jtbandes You'd spot that difference, if you do your own timings with the wall clock. e.g:

const start = Date.now(); // or performance for better granularity
call_heavy_setTimout_code_here();
console.log(Date.now() - start);

then do a run with profiling and a second one without devtools open. The difference between the the time taken will be almost all, what the profiler tells you as runtime for the setTimout stuff.

There is another domain, where devtools in chromium heavily screws up the timing data - heavy IO. Kinda learnt that the hard way scratching my head for some time, when optimizing IO throughput. I only figured it, when I accidently run things without devtools and was like "wut, why is that so much faster?". Turned out that most of the grey piece in the pie chart was made up by the profiler. I ended up doing most IO optimization with that "printf" profiling, as the profiler was not to be trusted, lol.

My best guess is, that the chromium profiler has some issues to correctly account runtime for threaded actions (most likely the timer and IO run in a different thread). I tried digging it up, but found nothing about that in v8 docs.

@Firefox
At least for heavy IO I could not repro this in firefox. Never tested it for setTimout...

@jtbandes
Copy link
Contributor

Using this TS playground I observe the following (Chrome 131.0.6778.140, macOS 15.2, M1 Pro):

(average over N=100k) setTimeout clearTimeout clearTimeout (reverse order) set+clear immediately
devtools closed 0.7µs 5µs 16µs 0.7µs
devtools open 3µs (4.2x) 5µs (1x) 16µs (1x) 2.7µs (3.8x)
during perf profile 3.8µs (5.4x) 8.5µs (1.7x) 21µs (1.3x) 7µs (10x)

It does seem like a pretty significant effect! However would still be nice to reduce these unnecessary calls :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/performance type/bug Something is misbehaving type/debt Technical debt that could slow us down in the long run
Projects
None yet
Development

No branches or pull requests

3 participants