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

perf: optimizing request plugin bundle resources #4639

Merged
merged 7 commits into from
Sep 27, 2023

Conversation

guqing
Copy link
Member

@guqing guqing commented Sep 21, 2023

What type of PR is this?

/kind improvement
/area core
/milestone 2.10.x

What this PR does / why we need it:

优化插件捆绑资源的查询性能

在插件 js bundle 1.2M 大小的情况下:
优化前:
image

优化后:
image

how to test it?
在插件生产模式下使用此 PR 测试,比对与不使用此 PR 的 API 速度,且检查 js bundle 和 css bundle 的资源是否正确加载。

Does this PR introduce a user-facing change?

优化插件捆绑资源(JSBundle)的查询性能以提高页面加载速度

@f2c-ci-robot f2c-ci-robot bot added kind/improvement Categorizes issue or PR as related to a improvement. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Sep 21, 2023
@f2c-ci-robot f2c-ci-robot bot added this to the 2.10.x milestone Sep 21, 2023
@f2c-ci-robot f2c-ci-robot bot requested review from ruibaby and wan92hen September 21, 2023 07:31
@f2c-ci-robot f2c-ci-robot bot added the area/core Issues or PRs related to the Halo Core label Sep 21, 2023
@guqing guqing requested a review from JohnNiang September 21, 2023 07:32
@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Merging #4639 (f58366b) into main (6106096) will increase coverage by 0.07%.
The diff coverage is 43.11%.

@@             Coverage Diff              @@
##               main    #4639      +/-   ##
============================================
+ Coverage     60.95%   61.02%   +0.07%     
  Complexity     2610     2610              
============================================
  Files           379      379              
  Lines         13551    13612      +61     
  Branches        961      960       -1     
============================================
+ Hits           8260     8307      +47     
- Misses         4827     4838      +11     
- Partials        464      467       +3     
Files Coverage Δ
...core/extension/service/impl/PluginServiceImpl.java 66.36% <0.00%> (+3.97%) ⬆️
...lo/app/core/extension/endpoint/PluginEndpoint.java 67.45% <56.62%> (+0.31%) ⬆️

@guqing guqing requested a review from LIlGG September 21, 2023 07:38
@ruibaby ruibaby changed the title perf: optimizing query for plugin bundle resources perf: optimizing request plugin bundle resources Sep 21, 2023
Copy link
Member

@ruibaby ruibaby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Sep 22, 2023
@LIlGG
Copy link
Member

LIlGG commented Sep 26, 2023

/lgtm

@guqing
Copy link
Member Author

guqing commented Sep 26, 2023

/ping @halo-dev/sig-halo

Copy link
Member

@JohnNiang JohnNiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里用 etag 能否解决问题呢?如果可以的话,可能就不需要缓存 js 和 css 到本地磁盘。

@guqing
Copy link
Member Author

guqing commented Sep 27, 2023

这里用 etag 能否解决问题呢?如果可以的话,可能就不需要缓存 js 和 css 到本地磁盘。

这里的问题与 http 缓存没有关系,主要是慢在每次访问bundle的时候需要去所有插件里面读取文件然后合并而去所有插件里面读取main.js和style.css的这个过程可以简化,作出的优化就是只合并一次写入到一个文件下次访问就只读取一个文件减少了IO次数,如果插件有变化那就再去重新读取并合并更新到文件以此类推

@JohnNiang JohnNiang modified the milestones: 2.10.x, 2.10.0 Sep 27, 2023
@JohnNiang
Copy link
Member

这里用 etag 能否解决问题呢?如果可以的话,可能就不需要缓存 js 和 css 到本地磁盘。

这里的问题与 http 缓存没有关系,主要是慢在每次访问bundle的时候需要去所有插件里面读取文件然后合并而去所有插件里面读取main.js和style.css的这个过程可以简化,作出的优化就是只合并一次写入到一个文件下次访问就只读取一个文件减少了IO次数,如果插件有变化那就再去重新读取并合并更新到文件以此类推

了解了。

@f2c-ci-robot f2c-ci-robot bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 27, 2023
@JohnNiang
Copy link
Member

似乎每次访问 http://localhost:8090/apis/api.console.halo.run/v1alpha1/plugins/-/bundle.js 都会重新写入文件。

@guqing
Copy link
Member Author

guqing commented Sep 27, 2023

似乎每次访问 http://localhost:8090/apis/api.console.halo.run/v1alpha1/plugins/-/bundle.js 都会重新写入文件。

要在生产模式下才有用, 开发模式下资源随时都会变所以不会缓存

@guqing guqing force-pushed the refactor/plugin-bundle branch from c2ff3c3 to 221fb47 Compare September 27, 2023 06:04
@guqing guqing requested a review from JohnNiang September 27, 2023 06:07
@guqing guqing requested a review from JohnNiang September 27, 2023 07:25
Copy link
Member

@JohnNiang JohnNiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Sep 27, 2023
@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Sep 27, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JohnNiang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@f2c-ci-robot f2c-ci-robot bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 27, 2023
@f2c-ci-robot f2c-ci-robot bot merged commit ce0e02a into halo-dev:main Sep 27, 2023
2 checks passed
@guqing guqing deleted the refactor/plugin-bundle branch September 27, 2023 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/core Issues or PRs related to the Halo Core kind/improvement Categorizes issue or PR as related to a improvement. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants