Performance issue on PRs with lots of changes

[eramdam]

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.

CleanShot.2023-11-27.at.17.27.02.mp4

How to replicate the issue + URL

Go to eramdam/BetterTweetDeck#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

[eramdam]

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]

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

[eramdam]

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.

[ishitatsuyuki]

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

[ishitatsuyuki]

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]

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]

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]

Sounds related to:

• Features are de-inited too late on ajax navigation #6437

[FloEdelmann]

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.

[eramdam]

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]

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]

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

using:

• ubuntu 23.10
• firefox developer edition version 121.0b7
• layout.css.has-selector.enabled: true

[fregante]

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

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

Side note: I must say Firefox' dev tools are junk-level.

[fregante]

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]

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]

I'm still seeing huge slowdowns on #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]

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]

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: #7192

[fregante]

Judging by the poor performance seen on the table-input widget in #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]

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]

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

[danillos]

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]

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]

probably be disabled via hotfix

Done

[github-actions]

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

[fregante]

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