dify icon indicating copy to clipboard operation
dify copied to clipboard

improve: significantly speed up the server launching time by async preloading tool providers

Open bowenliang123 opened this issue 1 year ago • 1 comments

Checklist:

[!IMPORTANT]
Please review the checklist below before submitting your pull request.

  • [x] Please open an issue before creating a PR or link to an existing issue
  • [x] I have performed a self-review of my own code
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [x] I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

Description

  • aync preloading the tool providers, which is thread-safe as it does check the status and locking (Tested on Apple MBP with M1pro chip)

  • Before: image

  • after: image

Type of Change

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] This change requires a documentation update, included: Dify Document
  • [x] Improvement, including but not limited to code refactoring, performance optimization, and UI/UX improvement
  • [ ] Dependency upgrade

Testing Instructions

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • [ ] Test A
  • [ ] Test B

bowenliang123 avatar Oct 10 '24 01:10 bowenliang123

It has been tested on the local machine. Greatly helpful for development use and shrinking the restart time for production re-deployment. cc @crazywoola @laipz8200

bowenliang123 avatar Oct 10 '24 01:10 bowenliang123

We plan to address this issue more thoroughly, so this PR will not be merged.

laipz8200 avatar Oct 14 '24 15:10 laipz8200

It's still worth considering as a quick fix with no side effects before a thorough refactoring.

bowenliang123 avatar Oct 29 '24 12:10 bowenliang123

Have you tried using separate threads to accomplish this function? In our environment, threads are managed by gevent, which helps maintain a consistent coding style.

laipz8200 avatar Oct 30 '24 01:10 laipz8200

In current implementation, asyncio's implicit event loop is used. It is dispatching an async task rather than holding a new dedicated thread to prevent over-design left unused for the rest time. As for the gevent , it would be considered if we do want a separate thread here, and also gevent's patching adapts the usages on standard library cooperative by default .

bowenliang123 avatar Oct 30 '24 01:10 bowenliang123

In current implementation, asyncio's implicit event loop is used. It is dispatching an async task rather than holding a new dedicated thread to prevent over-design left unused for the rest time. As for the gevent , it would be considered if we do want a separate thread here, and also gevent's patching adapts the usages on standard library cooperative by default .

Sorry for the misunderstanding. What I meant is that we can use the standard library’s threading here, and gevent will handle converting it to asynchronous behavior. I would prefer not to introduce the asyncio module at this point.

laipz8200 avatar Oct 30 '24 04:10 laipz8200

Well done! This startup acceleration is awesome.

yaoice avatar Oct 30 '24 14:10 yaoice

Have changed to use simple threading to execute the async task. Please have a review on it.

bowenliang123 avatar Oct 30 '24 15:10 bowenliang123

Thanks for the reconsideration and merging.

bowenliang123 avatar Oct 30 '24 17:10 bowenliang123