Skip to content

Commit

Permalink
Reduce memory footprint when parsing HTTP result
Browse files Browse the repository at this point in the history 
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)
  • Loading branch information
Gasol Wu committed Dec 14, 2018
1 parent bc1743a commit 951f76f
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 55 deletions.
127 changes: 103 additions & 24 deletions lib/Client.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,34 @@ class Client extends EventEmitter
*/
protected $maxRedirects = 5;

protected $headerLinesMap = [];

protected $beInherited;

/**
* Initializes the client.
*/
public function __construct()
{
$this->beInherited = __CLASS__ !== get_class($this);

$this->curlSettings = [
CURLOPT_RETURNTRANSFER => true,
CURLOPT_HEADER => true,
CURLOPT_NOBODY => false,
CURLOPT_USERAGENT => 'sabre-http/'.Version::VERSION.' (http://sabre.io/)',
];
if ($this->beInherited) {
$this->curlSettings[CURLOPT_HEADER] = true;
} else {
$this->curlSettings[CURLOPT_HEADERFUNCTION] = [$this, 'receiveCurlHeader'];
}
}

protected function receiveCurlHeader($curlHandle, $headerLine)
{
$this->headerLinesMap[(int) $curlHandle][] = $headerLine;

return strlen($headerLine);
}

/**
Expand Down Expand Up @@ -203,7 +220,9 @@ public function poll(): bool
$errorCallback,
$retryCount) = $this->curlMultiMap[$resourceId];
unset($this->curlMultiMap[$resourceId]);
$curlResult = $this->parseCurlResult(curl_multi_getcontent($status['handle']), $status['handle']);

$curlHandle = $status['handle'];
$curlResult = $this->parseResponse(curl_multi_getcontent($curlHandle), $curlHandle);
$retry = false;

if (self::STATUS_CURLERROR === $curlResult['status']) {
Expand Down Expand Up @@ -303,8 +322,7 @@ protected function doRequest(RequestInterface $request): ResponseInterface

curl_setopt_array($this->curlHandle, $settings);
$response = $this->curlExec($this->curlHandle);
$response = $this->parseCurlResult($response, $this->curlHandle);

$response = $this->parseResponse($response, $this->curlHandle);
if (self::STATUS_CURLERROR === $response['status']) {
throw new ClientException($response['curl_errmsg'], $response['curl_errno']);
}
Expand Down Expand Up @@ -397,6 +415,23 @@ protected function createCurlSettingsArray(RequestInterface $request): array
const STATUS_CURLERROR = 1;
const STATUS_HTTPERROR = 2;

private function parseResponse(string $response, $curlHandle): array
{
if ($this->beInherited) {
$response = $this->parseCurlResult($response, $curlHandle);
} else {
$resourceId = (int) $curlHandle;
if (isset($this->headerLinesMap[$resourceId])) {
$headers = $this->headerLinesMap[$resourceId];
} else {
$headers = [];
}
$response = $this->parseCurlResponse($headers, $response, $curlHandle);
}

return $response;
}

/**
* Parses the result of a curl call in a format that's a bit more
* convenient to work with.
Expand All @@ -412,6 +447,63 @@ protected function createCurlSettingsArray(RequestInterface $request): array
* * http_code - HTTP status code, as an int. Only set if Only set if
* status is STATUS_SUCCESS, or STATUS_HTTPERROR
*
* @param array $headerLines
* @param string $body
* @param resource $curlHandle
*/
protected function parseCurlResponse(array $headerLines, string $body, $curlHandle): array
{
list(
$curlInfo,
$curlErrNo,
$curlErrMsg
) = $this->curlStuff($curlHandle);

if ($curlErrNo) {
return [
'status' => self::STATUS_CURLERROR,
'curl_errno' => $curlErrNo,
'curl_errmsg' => $curlErrMsg,
];
}

$response = new Response();
$response->setStatus($curlInfo['http_code']);
$response->setBody($body);

foreach ($headerLines as $header) {
$parts = explode(':', $header, 2);
if (2 === count($parts)) {
$response->addHeader(trim($parts[0]), trim($parts[1]));
}
}

$httpCode = $response->getStatus();

return [
'status' => $httpCode >= 400 ? self::STATUS_HTTPERROR : self::STATUS_SUCCESS,
'response' => $response,
'http_code' => $httpCode,
];
}

/**
* Parses the result of a curl call in a format that's a bit more
* convenient to work with.
*
* The method returns an array with the following elements:
* * status - one of the 3 STATUS constants.
* * curl_errno - A curl error number. Only set if status is
* STATUS_CURLERROR.
* * curl_errmsg - A current error message. Only set if status is
* STATUS_CURLERROR.
* * response - Response object. Only set if status is STATUS_SUCCESS, or
* STATUS_HTTPERROR.
* * http_code - HTTP status code, as an int. Only set if Only set if
* status is STATUS_SUCCESS, or STATUS_HTTPERROR
*
* @deprecated Use parseCurlResponse instead
*
* @param resource $curlHandle
*/
protected function parseCurlResult(string $response, $curlHandle): array
Expand Down Expand Up @@ -449,25 +541,7 @@ protected function parseCurlResult(string $response, $curlHandle): array
// Splitting headers
$headerBlob = explode("\r\n", $headerBlob);

$response = new Response();
$response->setStatus($curlInfo['http_code']);

foreach ($headerBlob as $header) {
$parts = explode(':', $header, 2);
if (2 === count($parts)) {
$response->addHeader(trim($parts[0]), trim($parts[1]));
}
}

$response->setBody($responseBody);

$httpCode = $response->getStatus();

return [
'status' => $httpCode >= 400 ? self::STATUS_HTTPERROR : self::STATUS_SUCCESS,
'response' => $response,
'http_code' => $httpCode,
];
return $this->parseCurlResponse($headerBlob, $responseBody, $curlHandle);
}

/**
Expand All @@ -487,7 +561,10 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes
$this->createCurlSettingsArray($request)
);
curl_multi_add_handle($this->curlMultiHandle, $curl);
$this->curlMultiMap[(int) $curl] = [

$resourceId = (int) $curl;
$this->headerLinesMap[$resourceId] = [];
$this->curlMultiMap[$resourceId] = [
$request,
$success,
$error,
Expand All @@ -506,6 +583,8 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes
*/
protected function curlExec($curlHandle): string
{
$this->headerLinesMap[(int) $curlHandle] = [];

return curl_exec($curlHandle);
}

Expand Down
57 changes: 26 additions & 31 deletions tests/HTTP/ClientTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,24 @@ protected function getAbsoluteUrl($path)
return false;
}

/**
* @group ci
*/
public function testSendToGetLargeContent()
{
$url = $this->getAbsoluteUrl('/large.php');
if (!$url) {
$this->markTestSkipped('Set an environment value BASEURL to continue');
}

$request = new Request('GET', $url);
$client = new Client();
$response = $client->send($request);

$this->assertEquals(200, $response->getStatus());
$this->assertGreaterThan(memory_get_peak_usage(), 40 * pow(1024, 2));
}

/**
* @group ci
*/
Expand Down Expand Up @@ -369,37 +387,6 @@ public function testParseCurlResult()
$this->assertEquals('Foo', $result['response']->getBodyAsString());
}

/**
* @runInSeparateProcess
*/
public function testParseCurlLargeResult()
{
ini_set('memory_limit', '70M');

$header = "HTTP/1.1 200 OK\r\nHeader1: Val1\r\n\r\n";

$client = new ClientMock();
$client->on('curlStuff', function (&$return) use ($header) {
$return = [
[
'header_size' => strlen($header),
'http_code' => 200,
],
0,
'',
];
});

$body = str_repeat('x', 30 * pow(1024, 2));
$response = $header . $body;
$result = $client->parseCurlResult($response, 'foobar');

$this->assertEquals(Client::STATUS_SUCCESS, $result['status']);
$this->assertEquals(200, $result['http_code']);
$this->assertEquals(200, $result['response']->getStatus());
$this->assertEquals(31457280, strlen($result['response']->getBodyAsString()));
}

public function testParseCurlError()
{
$client = new ClientMock();
Expand Down Expand Up @@ -471,6 +458,14 @@ class ClientMock extends Client
{
protected $persistedSettings = [];

/**
* Making this method public.
*/
public function receiveCurlHeader($curlHandle, $headerLine)
{
return parent::receiveCurlHeader($curlHandle, $headerLine);
}

/**
* Making this method public.
*/
Expand Down
7 changes: 7 additions & 0 deletions tests/www/large.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

$data = str_repeat('x', 32 * 1024 * 1024);
header('Content-Length: '.strlen($data));
header('Content-Type: text/plain');

echo $data;

0 comments on commit 951f76f

Please sign in to comment.