refined-github / refined-github

:octocat: Browser extension that simplifies the GitHub interface and adds useful features
MIT License
25.13k stars 1.5k forks source link

Performance issue on PRs with lots of changes #7116

Closed eramdam closed 8 months ago

eramdam commented 1 year ago

Description

I wish I had a specific feature to point to, but since this issue seems to appear only when "first" loading a PR /files page, it was tricky for me to find anything conclusive with the troubleshooting tool. I'd be happy to run more diagnostics if necessary.

Basically, loading a big PR (+1179 -117 across 58 files on the example one) results in pretty slow loading/rendering and sometimes makes Firefox hang completely.

https://github.com/refined-github/refined-github/assets/1409924/79ec6e41-868d-4ed2-9068-bb19a89aa3f4

How to replicate the issue + URL

Go to https://github.com/eramdam/BetterTweetDeck/pull/519 then load the Files changed tab

Here are my settings: Refined GitHub options.json

Extension version

23.11.15.1433

Browser(s) used

Firefox 120.0 on macOS 14.1.1

fregante commented 1 year ago

Yes that page is slow even without Refined GitHub. Blame GitHub for making loading the entire diff in view

You can try disabling show-whitespace but it's otherwise somewhat optimized

eramdam commented 12 months ago

After testing a bit more I'm noticing that the difference in performance is much more noticeable on Firefox than Chrome so maybe there's something to dig into there (I thought I had written Firefox in my OP but i didn't, my bad 🤦 ). @fregante could you tell me if you notice the same thing? If not I can try to use the profiler and send the recording.

fregante commented 12 months ago

The question is: do you notice the same slowness after disabling Refined GitHub?

eramdam commented 12 months ago

The question is: do you notice the same slowness after disabling Refined GitHub?

Nope, disabling Refined GitHub makes the issue go away, but I agree that on Chrome it's way less noticeable 😅

Also I think having the files sidebar opened on a PR makes it worse but im not 100% sure.

ForsakenHarmony commented 12 months ago

@fregante I'm having the same issue one Firefox

Extension disabled: it's very usable, perf could maybe be a bit better, but it's fine.

Extension enabled: the whole page slows down to a crawl, half the page doesn't render and is just blank.

ishitatsuyuki commented 11 months ago

After bisection (Identify feature) quick-comment-edit seems to be the likely one that is causing the trouble.

ishitatsuyuki commented 11 months ago

Actually for what OP has linked locked-issue seems to be the culprit. quick-comment-edit seems to slow down pages with review comments on the other hand.

Both of them uses CSS animation based observers.

fregante commented 11 months ago

locked-issue is not enabled on the files tab so that's unlikely to be the cause. I'll check if there's some loop in the code of the observer but otherwise it should be lightweight.

Are you on Firefox too? Did you enable the has-selector option in about:config?

ishitatsuyuki commented 11 months ago

locked-issue is not enabled on the files tab so that's unlikely to be the cause. I'll check if there's some loop in the code of the observer but otherwise it should be lightweight.

It actually confirms the hypothesis more. When you load the files tab directly, there's no perf issue; when you load another tab then switch to files, it's very slow. I tested the latter during bisection.

fregante commented 11 months ago

Sounds related to:

ishitatsuyuki commented 11 months ago

It looks like has-selector definitely plays a role in this. The slow down doesn't happen if it's disabled.

has-selector is force enabled by one of the (rather popular) tweaks I use:

https://github.com/black7375/Firefox-UI-Fix/blob/431d32e29e58557a71b480f43cde10e371d012ec/user.js#L15

FloEdelmann commented 11 months ago

I can reproduce this as well. Disabling layout.css.has-selector.enabled in about:config fixes the performance issue. Disabling either quick-comment-edit or locked-issue or show-whitespace or all at once does not help.

lydell commented 11 months ago

I have layout.css.has-selector.enabled turned on as well, not because I toggled it but because I use Firefox Developer Edition which is always based on the upcoming version of Firefox, which currently is 121 and version 121 will enable :has by default: https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/121#css

I just tested, and I can confirm that turning off layout.css.has-selector.enabled (while making no other changes) makes the performance problem go away.

eramdam commented 11 months ago

FWIW, I did not have layout.css.has-selector.enabled turned on when recording my videos showing off the lag so I don't know if it's relevant to the issue at hand.

lydell commented 11 months ago

Maybe some of us are talking about one performance problem and some of us another. Quite the coincidence if there are two of them on the Files changed tab in Firefox at the same time 😄

For me, the Files changed is sluggish in general, but especially when I try to add a PR comment – it takes tens of seconds for the comment box to appear.

scarf005 commented 11 months ago

can confirm this slowdown also happens on job log. disabling quick-comment-edit also fixes it.

using:

fregante commented 11 months ago

The slow down doesn't happen if it's disabled.

That's because disabling :has also disables a bunch of features that require it.

fregante commented 11 months ago

The problem disappears for me when I disable locked-issue

I'm disabling the feature via hotfix for now, this is a bad bug for Firefox users.

It seems that it's somehow triggering GitHub's custom-elements events and even on the Conversation tab of that PR element.replaceWith is called 4000+ times. Unfortunately it seems to be impossible in Firefox to pin down where that call is located (🤦‍♂️) and in Chrome the issue does not appear in my case.

rostislav-simonik commented 11 months ago

I haven't investigated further, but when I'm on the PR Files changed tab and click the checkbox Viewed, I see performance degradation when the full extension is enabled. (using chrome)

fregante commented 11 months ago

I'm still seeing huge slowdowns on https://github.com/refined-github/refined-github/pull/6954 in Safari, even with locked-issue disabled. But bisect can't find the issue somehow, it points to CSS features. However if I disable all the features at once, it works well again.

cooljeanius commented 11 months ago

I've been seeing this while reviewing cooljeanius/Flight_Freedom#18 on Firefox 121... I'm wondering if it has something to do with me also having NoScript installed? Firefox has been sending me alternating "an extension is slowing down your browser" warnings about both Refined GitHub and NoScript, so I'm wondering if there's some sort of interaction between the two... locked-issue was already disabled for me, but the combination of disabling show-whitespace, quick-comment-edit, and quick-review-comment-deletion appears to have helped...

fregante commented 11 months ago

I'll accept a PR that disables those 4 features on !isChrome() && isPRFiles() as an interim solution.

I also opened another ticket for a longer-term solution: https://github.com/refined-github/refined-github/issues/7192

fregante commented 10 months ago

Judging by the poor performance seen on the table-input widget in https://github.com/refined-github/refined-github/pull/7211, I think this might be attributable to has selectors after all. But it's just a guess.

Turns out that letting the CSS handle it might not actually be the most efficient solution.

llucax commented 9 months ago

Just in case it is useful, for me show-whitespace seems to introduce some slowness, but it is a consistent one, everything is a little bit slower, but not unusable. While if I don't disable quick-comment-edit, it is really unusable, like in the video, where the whole tab is sort of locked and I can't even click somewhere else for a seconds or two (or three!).

Firefox 122.0 (64-bit), layout.css.has-selector.enabled=true .

carlosala commented 8 months ago

Here quick-comment-edit was the culprit as well, firefox 123.0 (64 bit). Removing show-whitespace helps a bit, as Leandro pointed 👍🏻

danillos commented 8 months ago

I'm having issues too.

MAC OS: Sonama (MacMini 3 GHz 6-Core Intel Core i5) Safari: Version 17.3 (19617.2.4.11.8)

Disabling show-whitespace, quick-comment-edit, and quick-review-comment-deletion appears to have helped too but didn't fix the issue just improves it.

When I disable the extension it opens faster.

Fun fact: When was I using MacOS Ventura I didn't have this issue, or it updated the extension on the same time

ncfavier commented 8 months ago

I could not open a diff at all on some PR until I turned off quick-comment-edit. The feature should probably be disabled via hotfix if it makes performance so bad.

fregante commented 8 months ago

probably be disabled via hotfix

Done

github-actions[bot] commented 8 months ago

To maintainers: Disable the hotfix by adding 24.3.25 to https://github.com/refined-github/yolo/edit/main/broken-features.csv

fregante commented 8 months ago

I'm closing this issue for now, I'll release a new version after #7313 is merged.