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

[Proposal] Optimize the index.js #23461

Closed
wxiaoguang opened this issue Mar 14, 2023 · 11 comments
Closed

[Proposal] Optimize the index.js #23461

wxiaoguang opened this issue Mar 14, 2023 · 11 comments
Labels
type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 14, 2023

Feature Description

This is an idea for public (feel free to take it or work on it).

Gitea's index.js logic becomes more and more complex. On my MacBook Pro, almost every page loading says:

[Violation] 'setTimeout' handler took 99ms
[Violation] 'setTimeout' handler took 148ms

I think the setTimeout is actually the $(document).ready(() => {}) call in index.js, it's too complex. If that $(document).ready() is commented out, there is no warning anymore.

So, I think in the future, it's better to avoid making the index.js do too much slow init calls, instead, many calls should be optimized (by various approaches)

Some clues

  • initHeatmap takes around 100ms (update: vue3-calendar-heatmap 2.0.2, the time decreases from 100ms to 50ms)
  • initGlobalCommon is also slow, if there are many elements
  • initRepoTopicBar seems unnecessarily slow
initHeatmap() 103.100 (update: vue3-calendar-heatmap 2.0.2, the time decreases from 100ms to 50ms)
initGlobalCommon() 20.800
initDashboardRepoList() 12.400
initGlobalTooltips() 8.900
initGlobalFormDirtyLeaveConfirm() 1.400
initStopwatch() 1.100
initMarkupContent() 1.000
initRepoTopicBar() 0.600
attachTribute(document.querySelectorAll('#content, .emoji-input')) 0.400
initNotificationCount() 0.400
initMarkupAnchors() 0.300
initServiceWorker() 0.300
initRepoIssueList() 0.300
initGlobalCommon() 33.900
initRepoTopicBar() 32.300
initRepository() 10.600
initGlobalTooltips() 8.200
initGlobalFormDirtyLeaveConfirm() 1.600
initStopwatch() 0.700
initMarkupContent() 0.600
initMarkupAnchors() 0.300
attachTribute(document.querySelectorAll('#content, .emoji-input')) 0.300
@wxiaoguang wxiaoguang added type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Mar 14, 2023
@silverwind
Copy link
Member

silverwind commented Mar 14, 2023

$(document).ready(() => {})

Can probably replace ready with DOMContentLoaded, to lessen our jQuery dependency. Oh, and the syntax in use is apparently deprecated as of jQuery 3.0.

initHeatmap takes around 100ms.

Is this true for pages without a heatmap as well? If so, we should definitely add early exit to that function. Generally it should be noted that heatmap data generation is very inefficient on server-side too.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Mar 14, 2023

$(document).ready(() => {})

Can probably replace ready with DOMContentLoaded, to lessen our jQuery dependency. Oh, and the syntax in use is apparently deprecated as of jQuery 3.0.

I agree we can refactor, but It's not related to this problem. The problem is many init functions are really slow.

initHeatmap takes around 100ms.

Is this true for pages without a heatmap as well? If so, we should definitely add early exit to that function. Generally it should be noted that heatmap data generation is very inefficient on server-side too.

No initHeatmap, no 100ms. With initHeatmap, then 100ms.

Other init functions could also be slow, eg: initGlobalCommon and initRepoTopicBar could also take 30ms

You could take a try.

  const fns = [];

  let last = performance.now();
  function record(s) {
    const now = performance.now();
    fns.push({fn:s, t: now-last});
    last = now;
  }

  initGlobalCommon();record(`initGlobalCommon()`);
...
  initUserSettings();record(`initUserSettings()`);
  initViewedCheckboxListenerFor();record(`initViewedCheckboxListenerFor()`);

  fns.sort((a, b) => b.t - a.t).forEach((f) => console.log(f.fn, f.t.toFixed(3)));

@lunny
Copy link
Member

lunny commented Mar 14, 2023

Can we split index.js, at least we can have an admin.js which will only include admin related js

@wxiaoguang
Copy link
Contributor Author

Can we split index.js, at least we can have an admin.js which will only include admin related js

At the moment, initAdmin related functions only cost 0~0.1ms. See my test code above.

Find the root problem and focus on the root problem.

@HesterG
Copy link
Contributor

HesterG commented Mar 15, 2023

Which measurement tool did you use?

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Mar 15, 2023

Which measurement tool did you use?

Home-made JS in #23461 (comment)


I push the test code to https://github.com/wxiaoguang/gitea/tree/test-js-slow

@HesterG
Copy link
Contributor

HesterG commented Mar 15, 2023

截屏2023-03-15 16 36 17

For Heatmap, looks like it is depending on tippy, which takes the half of the init time.

@silverwind
Copy link
Member

silverwind commented Mar 15, 2023

Note this is a separate, second instance of tippy.js because https://github.com/razorness/vue3-calendar-heatmap has its own dependency on it. So essentially we init tippy twice because of it. I guess a better solution may be to incorporate that module into first-party code.

@silverwind
Copy link
Member

silverwind commented Mar 16, 2023

Looking through that module, it seem to create a tippy instance for each day, so 365 tippy instances. I think this is reason why it's slow to init. A better approach may be to just use title attribute or to only create instances on-demand on mouseenter/click event.

@wxiaoguang
Copy link
Contributor Author

vue3-calendar-heatmap has released 2.0.2 with lazy tippy init.

According to my test, the initHeatmap time decreases from 100ms to 50ms. So I put the upgrading in #23574 together.

@wxiaoguang
Copy link
Contributor Author

Out-dated and there is no planned work in near future.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

4 participants