Skip to content

Commit

Permalink
Remove direct dependency on amphp/file, improve CPU usage and perf
Browse files Browse the repository at this point in the history
amphp/file was replaced with ResourceInputStream, which might result
in blocking operations, however so far I did not notice any.

Following removal of amphp/file dependency in amphp/dns (at d26f9bb),
this change should (when new version of amphp/dns is released) result in
eliminating requirement of 3 indirect dependencies.

This change improves:
- CPU utilization (from ~51% to ~35%)
- max speed (from 78.8MiB/s to 170MiB/s)

Given stdin `head -c 1G </dev/urandom | pv -q -L 10000000`:

Before:
      83205.359732      task-clock (msec)     #    0.776 CPUs utilized
           279,904      context-switches      #    0.003 M/sec
            15,297      cpu-migrations        #    0.184 K/sec
             5,318      page-faults           #    0.064 K/sec
    79,455,374,201      cycles                #    0.955 GHz
    44,104,516,695      instructions          #    0.56  insn per cycle
     9,306,984,820      branches              #  111.856 M/sec
       452,310,830      branch-misses         #    4.86% of all branches

     107.261390647 seconds time elapsed

After:
      37608.420764      task-clock (msec)     #    0.350 CPUs utilized
            27,243      context-switches      #    0.724 K/sec
             2,032      cpu-migrations        #    0.054 K/sec
             3,261      page-faults           #    0.087 K/sec
    17,073,590,870      cycles                #    0.454 GHz
    12,015,172,856      instructions          #    0.70  insn per cycle
     2,428,621,990      branches              #   64.577 M/sec
        64,394,486      branch-misses         #    2.65% of all branches

     107.449499919 seconds time elapsed

Given `head -c 1G </dev/urandom`:

Before:
      18344.168358      task-clock (msec)     #    1.284 CPUs utilized
           248,097      context-switches      #    0.014 M/sec
             3,901      cpu-migrations        #    0.213 K/sec
             5,285      page-faults           #    0.288 K/sec
    62,863,094,352      cycles                #    3.427 GHz
    42,953,043,093      instructions          #    0.68  insn per cycle
     9,069,974,709      branches              #  494.434 M/sec
       308,754,921      branch-misses         #    3.40% of all branches

      14.290214331 seconds time elapsed

After:
       4913.930543      task-clock (msec)     #    0.737 CPUs utilized
            74,337      context-switches      #    0.015 M/sec
             1,105      cpu-migrations        #    0.225 K/sec
             3,256      page-faults           #    0.663 K/sec
    16,824,387,362      cycles                #    3.424 GHz
    12,426,816,219      instructions          #    0.74  insn per cycle
     2,545,396,441      branches              #  517.996 M/sec
        46,043,534      branch-misses         #    1.81% of all branches

       6.666760699 seconds time elapsed
  • Loading branch information
ostrolucky committed Jan 12, 2019
1 parent d3b3ad6 commit a290135
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 10 deletions.
1 change: 0 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
"ext-fileinfo": "*",
"amphp/amp": "^2.0",
"amphp/byte-stream": "^1.2.5",
"amphp/file": "^0.2 || ^0.3",
"amphp/socket": "^0.10",
"jean85/pretty-package-versions": "^1.1",
"psr/log": "^1.0",
Expand Down
31 changes: 22 additions & 9 deletions src/Responder.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

namespace Ostrolucky\Stdinho;

use Amp\ByteStream\ResourceInputStream;
use Amp\ByteStream\StreamException;
use Amp\File\Handle;
use Amp\Socket\Socket;
use Ostrolucky\Stdinho\Bufferer\BuffererInterface;
use Psr\Log\LoggerInterface;
Expand All @@ -27,9 +27,6 @@ public function __invoke(Socket $socket): \Generator
$remoteAddress = $socket->getRemoteAddress();
$this->logger->debug("Accepted connection from $remoteAddress:\n" . trim(yield $socket->read()));

/** @var Handle $handle */
$handle = yield \Amp\File\open($this->bufferer->getFilePath(), 'rb');

$header = [
'HTTP/1.1 200 OK',
'Content-Type:' . yield $this->bufferer->getMimeType(),
Expand All @@ -49,26 +46,42 @@ public function __invoke(Socket $socket): \Generator
$remoteAddress
);

$handle = new ResourceInputStream(fopen($this->bufferer->getFilePath(), 'rb'));

try {
while (($chunk = yield $handle->read()) || $this->bufferer->isBuffering()) {
// we reached end of the buffer, but it's still buffering
if ($chunk === null) {
while (true) {
$buffererProgress = $this->bufferer->getCurrentProgress();

/**
* Not trying to read the buffer when connected client caught up to it is important for following:
*
* 1. Reduce CPU usage and potential block operations thanks to avoiding executing logic in read()
* 2. Prevent ResourceInputStream to close the resource when it detects feof. This serves as workaround.
*
* @see https://github.com/ostrolucky/stdinho/pull/2
* @see https://github.com/amphp/byte-stream/issues/47
*/
if ($buffererProgress <= $progressBar->step && $this->bufferer->isBuffering()) {
yield $this->bufferer->waitForWrite();
continue;
}

if (($chunk = yield $handle->read()) === null) {
break; // No more buffering and client caught up to it -> finish download
}

yield $socket->write($chunk);

$progressBar->max = $this->bufferer->getCurrentProgress();
$progressBar->advance(strlen($chunk));
};
}
$progressBar->finish();
$this->logger->debug("$remoteAddress finished download");
} catch (StreamException $exception) {
$this->logger->debug("$remoteAddress aborted download");
}

$handle->end();
$handle->close();
$socket->end();
}
}

0 comments on commit a290135

Please sign in to comment.