Reduce memory footprint when parsing HTTP result #115

Gasol · 2018-12-12T16:06:23Z

PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 48318938 bytes) in /app/vendor/sabre/http/lib/Client.php on line 498

In order to resolve #82 to avoid additional memory copy of response body (from $response to $responseBody), We deperecated parseCurlResult method which use substr function to separate HTTP headers and body and create new
method parseCurlResponse instead. We register receiveCurlHeader method as callback function by curl_setopt using CURLOPT_HEADERFUNCTION option. See #115 (comment).

This changes is not intended to resolve issue #15

codecov · 2018-12-12T16:10:37Z

Codecov Report

Merging #115 into master will increase coverage by 5.29%.
The diff coverage is 90.47%.

@@             Coverage Diff             @@
##             master    #115      +/-   ##
===========================================
+ Coverage     88.21%   93.5%   +5.29%     
- Complexity      242     250       +8     
===========================================
  Files            15      15              
  Lines           789     816      +27     
===========================================
+ Hits            696     763      +67     
+ Misses           93      53      -40

Impacted Files	Coverage Δ	Complexity Δ
lib/Client.php	`83.85% <90.47%> (+26.88%)`	`57 <10> (+8)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e55cc6...3fe1452. Read the comment docs.

staabm · 2018-12-12T16:23:34Z

Really cool. Did you measure how much memory consumption improves with this fix?

lib/Client.php

Gasol · 2018-12-13T01:44:05Z

Really cool. Did you measure how much memory consumption improves with this fix?

It save 1x memory copy depends on size of HTTP body.

<?php

ini_set('memory_limit', '128M');

$response = str_repeat('a', 100 * pow(1024, 2));
echo 'peak usage: ' . memory_get_peak_usage() . PHP_EOL;
$responseBody = substr($response, 1); // Get called in parseCurlResult()
echo 'peak usage: ' . memory_get_peak_usage() . PHP_EOL;

Output

peak usage: 105271376
PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 104857624 bytes) in /net/enterprise-data1/home/admin/gasolwu/Code/kklibs-img/str.php on line 7
PHP Stack trace:
PHP   1. {main}() /home/gasolwu/Code/substr.php:0
PHP   2. substr() /home/gasolwu/Code/substr.php:7

Would throw an excpetion like following message PHPUnit\Framework\Exception: PHP Fatal error: Allowed memory size of 73400320 bytes exhausted (tried to allocate 31457312 bytes) in /home/gasolwu/Code/sabre-http/lib/Client.php on line 437

In order to resolve sabre-io#82 to avoid additional memory copy of response body (from $response to $responseBody), We deperecated `parseCurlResult` method which use `substr` function to separate HTTP headers and body and create new method `parseCurlResponse` instead. We register `receiveCurlHeader` method as callback function by curl_setopt using `CURLOPT_HEADERFUNCTION` option. This changes is not intended to resolve issue sabre-io#15 See sabre-io#115 (comment)

Let PHPUnit be quiet PHPUnit 7.5.1 by Sebastian Bergmann and contributors. Warning - The configuration file did not pass validation! The following problems have been detected: Line 8: - Element 'phpunit', attribute 'strict': The attribute 'strict' is not allowed. Test results may not be as expected. See sebastianbergmann/phpunit@78c8865

The feature needs to install additional package. Error: Package phpunit/php-invoker is required for enforcing time limits

In order to resolve sabre-io#82 to avoid additional memory copy of response body (from $response to $responseBody), We deperecated `parseCurlResult` method which use `substr` function to separate HTTP headers and body and create new method `parseCurlResponse` instead. We register `receiveCurlHeader` method as callback function by curl_setopt using `CURLOPT_HEADERFUNCTION` option. This changes is not intended to resolve issue sabre-io#15 See sabre-io#115 (comment)

Use local variable instead

Fix error like TypeError: Return value of Sabre\HTTP\Client::curlExec() must be of the type string, boolean returned

Gasol · 2019-01-11T02:05:38Z

any concerns?

We are using inline alias for temporary fixes for a while, It is painful to maintain forked project on internal VCS hosting.

staabm

Sorry, totally forgot this PR

In order to resolve #82 to avoid additional memory copy of response body (from $response to $responseBody), We deperecated `parseCurlResult` method which use `substr` function to separate HTTP headers and body and create new method `parseCurlResponse` instead. We register `receiveCurlHeader` method as callback function by curl_setopt using `CURLOPT_HEADERFUNCTION` option. This changes is not intended to resolve issue #15 See #115 (comment)

staabm · 2019-03-18T18:34:25Z

Thx. Sorry for the long wait periode.

Well done!

Gasol force-pushed the headerfunction branch from 24fbf06 to 8f227d1 Compare December 12, 2018 16:10

staabm approved these changes Dec 12, 2018

View reviewed changes

staabm reviewed Dec 12, 2018

View reviewed changes

lib/Client.php Outdated Show resolved Hide resolved

staabm reviewed Dec 12, 2018

View reviewed changes

lib/Client.php Outdated Show resolved Hide resolved

Gasol force-pushed the headerfunction branch 2 times, most recently from 48020a2 to ef10ff6 Compare December 13, 2018 02:57

Gasol changed the title ~~Reduce memory footprint when parsing HTTP result~~ WIP: Reduce memory footprint when parsing HTTP result Dec 13, 2018

Test parseCurlResult with large response

dbb7c46

Would throw an excpetion like following message PHPUnit\Framework\Exception: PHP Fatal error: Allowed memory size of 73400320 bytes exhausted (tried to allocate 31457312 bytes) in /home/gasolwu/Code/sabre-http/lib/Client.php on line 437

Gasol force-pushed the headerfunction branch 3 times, most recently from 6f31a50 to d41058d Compare December 13, 2018 11:04

Gasol force-pushed the headerfunction branch from d41058d to 0f657da Compare December 14, 2018 03:32

Gasol Wu added 3 commits December 14, 2018 11:38

Test sendAsync using PHP built-in web server in CI

a08e6b4

Remove enforceTimeLimit attribute

bc1743a

The feature needs to install additional package. Error: Package phpunit/php-invoker is required for enforcing time limits

Gasol force-pushed the headerfunction branch from 0f657da to 74ebd58 Compare December 14, 2018 03:39

Gasol changed the title ~~WIP: Reduce memory footprint when parsing HTTP result~~ Reduce memory footprint when parsing HTTP result Dec 14, 2018

Gasol force-pushed the headerfunction branch from 74ebd58 to 4e1daa6 Compare December 14, 2018 04:21

Eliminate property $beInherited

3811b56

Use local variable instead

Gasol force-pushed the headerfunction branch from 4e1daa6 to ab73d45 Compare December 14, 2018 06:39

Ensure a string return by curlExec

3fe1452

Fix error like TypeError: Return value of Sabre\HTTP\Client::curlExec() must be of the type string, boolean returned

Gasol force-pushed the headerfunction branch from ab73d45 to 3fe1452 Compare December 14, 2018 06:56

staabm approved these changes Jan 11, 2019

View reviewed changes

staabm requested a review from DeepDiver1975 January 11, 2019 06:16

evert approved these changes Mar 18, 2019

View reviewed changes

staabm merged commit 544c67e into sabre-io:master Mar 18, 2019

Gasol deleted the headerfunction branch March 19, 2019 01:36

VicDeo mentioned this pull request Sep 11, 2019

A new release please #122

Closed

phil-davis mentioned this pull request Sep 12, 2019

parseCurlResponse() cannot take NULL body #126

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduce memory footprint when parsing HTTP result #115

Reduce memory footprint when parsing HTTP result #115

Gasol commented Dec 12, 2018 •

edited

Loading

codecov bot commented Dec 12, 2018 •

edited

Loading

staabm commented Dec 12, 2018

Gasol commented Dec 13, 2018

Gasol commented Jan 11, 2019

staabm left a comment

staabm commented Mar 18, 2019

Reduce memory footprint when parsing HTTP result #115

Reduce memory footprint when parsing HTTP result #115

Conversation

Gasol commented Dec 12, 2018 • edited Loading

codecov bot commented Dec 12, 2018 • edited Loading

Codecov Report

staabm commented Dec 12, 2018

Gasol commented Dec 13, 2018

Gasol commented Jan 11, 2019

staabm left a comment

Choose a reason for hiding this comment

staabm commented Mar 18, 2019

Gasol commented Dec 12, 2018 •

edited

Loading

codecov bot commented Dec 12, 2018 •

edited

Loading