-
Notifications
You must be signed in to change notification settings - Fork 52
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
[Fix] Release resources after use #133
Merged
juncaipeng
merged 11 commits into
PaddlePaddle:develop
from
Bobholamovic:fix/release_resource
Dec 22, 2023
Merged
[Fix] Release resources after use #133
juncaipeng
merged 11 commits into
PaddlePaddle:develop
from
Bobholamovic:fix/release_resource
Dec 22, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@juncaipeng 看吧,这个PR我没有context |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #133 +/- ##
===========================================
- Coverage 61.83% 61.45% -0.38%
===========================================
Files 56 56
Lines 2780 2771 -9
===========================================
- Hits 1719 1703 -16
- Misses 1061 1068 +7 ☔ View full report in Codecov by Sentry. |
juncaipeng
approved these changes
Dec 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
本PR主要解决
erniebot/http_client.py
中的两个资源管理相关问题:目前
erniebot
库主要提供函数式接口,但出于性能考虑对requests session进行了缓存,这导致资源泄漏(无用的session在内存中长期存在)。本PR重新设计了erniebot
库的session部分:当用户未进行任何额外配置时,默认为每个request新建一个session,此时调用方式简单但性能较差,适合用于做demo;当用户配置requests_session
和aiohttp_session
时,支持复用requests/aiohttp会话,可以在多个EBClient
对象乃至全局层面共享会话,从而提升性能。该设计的一些选择和主要考虑如下:EBClient
不管理session资源的申请和释放,也就是说它view而不是own session。这主要是出于如下考虑:_config_
。而如果我们让EBClient
own session,局部的session变得相对没意义(毕竟每次传入的session只会用在一次请求),全局session的情况就会变得复杂(我们需要确保EBClient
对全局和局部的session的处理逻辑不一样,全局session不会在一次请求后就被销毁)。虽然在函数式接口以外推广一套面向对象的接口以自动管理session也是一个可选项,但就项目目前的情况而言,这种做法的开发成本较高,对现存用户代码的影响面也较大。EBClient
对象级别,因为每个resource对象持有一个client)session的目的主要是在不同的request之间共享session,而这些session实际上也是可以在更大的范围内共享的,例如全局共享或是一个backend的不同resource之间共享(我们假设应用中每次可能更倾向于只用到一种resource,因此每个resource都是单独的类,采用单resource可选多backend的设计而不是单client集成多resource的设计),EBClient
对象中不释放资源有助于开发者在外部进行更精细化的资源共享配置,只是需要开发者自行进行资源的申请和释放。可以说,viewing object更适配当前的resource类设计。完成HTTP请求后没有及时关闭连接,在并发较高的情况下,这可能导致连接池资源耗尽。本PR修复了这个问题。