Skip to content
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

Delay when using Filesystem compared to ReadableResourceStream directly #85

Open
jeromegamez opened this issue Jul 20, 2020 · 3 comments

Comments

@jeromegamez
Copy link

Greetings 👋,

I'm fairly new to ReactPHP, please forgive me if this is something obvious and my Google/Stack Overflow search skills have just failed me 🙈.

"react/filesystem": "^0.1.2" is the only dependency in my composer.json, and I'm using PHP 7.4.8 installed with Homebrew on MacOS Big Sur (I hope that's not the reason 😅), with the StreamSelectLoop (no event extensions are installed).

The following code hopefully makes it reproducible:

// test.php
use React\EventLoop\Factory;
use React\Filesystem\Filesystem;
use React\Stream\ReadableResourceStream;

$loop = Factory::create();

$filePath = 'test.txt';

$file = new ReadableResourceStream(fopen($filePath, 'r'), $loop);
$fs = Filesystem::create($loop);

When using the ReadableResourceStream, the text is printed and the script terminates immediately:

$file->on('data', function ($contents) {
    echo $contents;
});

$loop->run();
time php test.php
test
php test.php  0,04s user 0,03s system 94% cpu 0,070 total

When using the Filesystem, the text is printed immediately, but it takes another three to four seconds until the script finally terminates:

$fs->getContents($filePath)
    ->then(function ($contents) {
        echo $contents;
    });

$loop->run();
time php test.php
test
php test.php  0,15s user 0,11s system 6% cpu 4,083 total

I'm not sure if this is an actual problem or if I missed something in the docs - either way I'd be happy if you could give me a hint on what might be going on here. (I also noticed that the system time is quite different between the two methods)

:octocat:

@WyriHaximus
Copy link
Member

This isn't so much as a bug, but as a consequence of child processes or threads to do the IO. It takes a bit to shut them down, where with you first example you use a slightly blocking method of opening files. ALso this is something that shouldn't be an issue with long running processes :)

@clue
Copy link
Member

clue commented Aug 12, 2020

@jeromegamez Thanks for reporting, I agree that this is definitely unexpected behavior! 👍

@WyriHaximus I understand the technical background, but do we really need to keep these processes around when no other work is outstanding? IMHO we should default to a very small timeout (1ms) with the option of increasing this for long-running processes. Of course spawning child processes incurs some overhead, but I'd rather solve the 80% use case and then look into additional optimizations for more specific use cases. What do you think? 👍

@WyriHaximus
Copy link
Member

@clue makes sense 👍 . Will have a look at this soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants