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

native curl socket optimize #5110

Closed
wants to merge 4 commits into from
Closed

native curl socket optimize #5110

wants to merge 4 commits into from

Conversation

shiguangqi
Copy link
Member

support native curl persistent connection

@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #5110 (fa6da9b) into master (5cfa1db) will decrease coverage by 0.09%.
Report is 3 commits behind head on master.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5110      +/-   ##
==========================================
- Coverage   73.06%   72.98%   -0.09%     
==========================================
  Files          69       69              
  Lines       14845    14845              
==========================================
- Hits        10846    10834      -12     
- Misses       3999     4011      +12     

see 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -2303,6 +2303,7 @@ PHP_FUNCTION(swoole_native_curl_close) {
RETURN_FALSE;
}

swoole::curl::destroy_handle(ch->cp);
Copy link
Member

Choose a reason for hiding this comment

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

应该在最下方

@@ -35,6 +35,7 @@ Handle *create_handle(CURL *cp) {
return nullptr;
}
Handle *handle = new Handle(cp);
handle->curl = new Multi();
Copy link
Member

Choose a reason for hiding this comment

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

假如这个 handler 是配合 multi 使用的,这里创建的 Multi 无意义

@shiguangqi shiguangqi closed this Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants