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, Remove the substr function in
`parseCurlResult()` and register receiveCurlHeader as callback function by
using CURLOPT_HEADERFUNCTION instead, To avoid additional memory copy
of response body (from $response to $responseBody).

This changes is not intended to resolve issue sabre-io#15
  • Loading branch information
Gasol Wu committed Dec 13, 2018
1 parent 008c80d commit ad25c39
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 34 deletions.
75 changes: 72 additions & 3 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 @@ -303,7 +320,17 @@ protected function doRequest(RequestInterface $request): ResponseInterface

curl_setopt_array($this->curlHandle, $settings);
$response = $this->curlExec($this->curlHandle);
$response = $this->parseCurlResult($response, $this->curlHandle);
if ($this->beInherited) { // backward compatible
$response = $this->parseCurlResult($response, $this->curlHandle);
} else {
$resourceId = (int) $this->curlHandle;
if (isset($this->headerLinesMap[$resourceId])) {
$headers = $this->headerLinesMap[$resourceId];
} else {
$headers = [];
}
$response = $this->parseCurlResponse($headers, $response, $this->curlHandle);
}

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

protected function parseCurlResultResponse(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.
Expand All @@ -412,6 +475,7 @@ 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
*
* @deprecated Use parseCurlResultResponse instead
* @param resource $curlHandle
*/
protected function parseCurlResult(string $response, $curlHandle): array
Expand Down Expand Up @@ -487,7 +551,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 +573,8 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes
*/
protected function curlExec($curlHandle): string
{
$this->headerLinesMap[(int) $curlHandle] = [];

return curl_exec($curlHandle);
}

Expand Down
41 changes: 10 additions & 31 deletions tests/HTTP/ClientTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,37 +367,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 @@ -467,8 +436,18 @@ public function testDoRequestCurlError()

class ClientMock extends Client
{
public $beInherited = true;

protected $persistedSettings = [];

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

/**
* Making this method public.
*/
Expand Down

0 comments on commit ad25c39

Please sign in to comment.