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

refactor(client): remove goroutine synchronization in DoDeadline #681

Merged
merged 4 commits into from
Mar 24, 2023

Conversation

Duslia
Copy link
Member

@Duslia Duslia commented Mar 20, 2023

What type of PR is this?

refactor

Check the PR title.

  • This PR title match the format: <type>(optional scope): <description>
  • The description of this PR title is user-oriented and clear enough for others to understand.

(Optional) Translate the PR title into Chinese.

去除 DoDeadline 中的 goroutine 同步

(Optional) More detail description for this PR(en: English/zh: Chinese).

en: Refactor client.DoDeadline, control the timeout through dialtimeout, writetimeout, readtimeout, reduce the number of goroutines and copies
zh(optional): 重构 client.DoDeadline,通过 dialtimeout、writetimeout、readtimeout 控制超时,减少协程和拷贝数量

Which issue(s) this PR fixes:

@Duslia Duslia requested review from a team as code owners March 20, 2023 13:47
@Duslia Duslia force-pushed the refactor/deadline branch from db1d596 to 952667c Compare March 21, 2023 02:07
@Duslia Duslia changed the title [wip]refactor: deadline refactor: deadline Mar 21, 2023
@Duslia Duslia force-pushed the refactor/deadline branch 3 times, most recently from e7c3da3 to 7e69b7a Compare March 21, 2023 02:15
@codecov
Copy link

codecov bot commented Mar 21, 2023

Codecov Report

Patch coverage: 45.45% and project coverage change: -0.13 ⚠️

Comparison is base (5f0145e) 74.30% compared to head (a5b74db) 74.18%.

❗ Current head a5b74db differs from pull request most recent head cb3ed7f. Consider uploading reports for the commit cb3ed7f to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #681      +/-   ##
===========================================
- Coverage    74.30%   74.18%   -0.13%     
===========================================
  Files           96       96              
  Lines         9392     9433      +41     
===========================================
+ Hits          6979     6998      +19     
- Misses        1982     1999      +17     
- Partials       431      436       +5     
Impacted Files Coverage Δ
pkg/protocol/request.go 83.85% <ø> (ø)
pkg/common/config/request_option.go 83.82% <15.38%> (-16.18%) ⬇️
pkg/common/test/mock/network.go 89.84% <30.00%> (-7.72%) ⬇️
pkg/protocol/http1/client.go 58.56% <62.50%> (+1.64%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Duslia Duslia force-pushed the refactor/deadline branch 2 times, most recently from a372aba to 294b6cb Compare March 21, 2023 02:50
pkg/protocol/request.go Outdated Show resolved Hide resolved
@Duslia Duslia force-pushed the refactor/deadline branch 2 times, most recently from e4a599d to ebbce6d Compare March 23, 2023 08:20
@Duslia Duslia force-pushed the refactor/deadline branch 2 times, most recently from 0d0588d to 2d8c0ca Compare March 23, 2023 09:46
@Duslia
Copy link
Member Author

Duslia commented Mar 23, 2023

benchamrk:
previous version
image
improve version:
HKMozTnkm8

@Duslia Duslia force-pushed the refactor/deadline branch from 2d8c0ca to d0b1f98 Compare March 23, 2023 11:53
@Duslia Duslia force-pushed the refactor/deadline branch 3 times, most recently from 5991377 to 447314d Compare March 23, 2023 12:07
@welkeyever welkeyever changed the title refactor: deadline refactor(client): remove goroutine synchronization in DoDeadline Mar 23, 2023
@Duslia Duslia force-pushed the refactor/deadline branch from 447314d to cb3ed7f Compare March 23, 2023 13:03
@Duslia Duslia merged commit 7faf99a into cloudwego:develop Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants