-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpClient] Improve memory consumption #60241
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
Conversation
hello @nicolas-grekas 👋🏻 |
44a5e56
to
7212515
Compare
It's related yes: the same approach could be taken for all traceable decorators. Help wanted for the others /cc @lyrixx et al. |
08fe94b
to
d2bcf96
Compare
6a518ff
to
b102057
Compare
) { | ||
$this->tracedRequests = new \ArrayObject(); | ||
} | ||
|
||
public function enabled(?bool $enabled = null): bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(the class is final already)
src/Symfony/Component/HttpClient/DataCollector/HttpClientDataCollector.php
Outdated
Show resolved
Hide resolved
b102057
to
0e865e6
Compare
On the CLI, even if the profiler is not enabled by default, the underlying infrastructure is still in place.
In this PR, I'm targeting TraceableHttpClient, which collects requests and responses for nothing.
To fix this, I'm adding an "enabled" flag on TraceableHttpClient, so that we can enable the tracing only when the profiler is enabled.
In addition, I'm augmenting option
extra.trace_content
so that when disabled, we don't collect request's bodies, which can be huge when uploading.And last but not least, I'm fixing the implementation of curl's debug info collection, which currently relies on allocating new strings all the time instead of reusing the already created zval when debug info didn't change.