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

Tapping on a very active .out of Proc::Async wrecks the work queue #5894

Open
p6rt opened this issue Dec 18, 2016 · 4 comments
Open

Tapping on a very active .out of Proc::Async wrecks the work queue #5894

p6rt opened this issue Dec 18, 2016 · 4 comments

Comments

@p6rt
Copy link

@p6rt p6rt commented Dec 18, 2016

Migrated from rt.perl.org#130370 (status was 'open')

Searchable as RT130370$

@p6rt
Copy link
Author

@p6rt p6rt commented Dec 18, 2016

From @AlexDaniel

This is probably best demonstrated by a code snippet.

Code​:

my $out = Channel.new;
#my $proc = Proc​::Async.new(‘perl6’, ‘-e’, ‘sleep ∞’); # ← this works
my $proc = Proc​::Async.new(‘perl6’, ‘-e’, ‘.say for 0..0x1FFFF’); # ← this doesn't
$proc.stdout.tap({ $out.send​: 1 });

my $promise = $proc.start;
say ‘let's await ’, now;
await Promise.anyof(Promise.in(1), $promise);
say ‘let's close ’, now;
$proc.kill;
await $promise;
$out.close;
say ‘that's it ’, now;

The idea is that we want to kill our process if it is still working after 1 second. This works fine, unless the process is writing a lot of stuff to stdout.

IRC log​: https://irclog.perlgeek.de/perl6/2016-12-18#i_13760994

Loading

@p6rt
Copy link
Author

@p6rt p6rt commented Dec 20, 2016

From @jnthn

On Sun, 18 Dec 2016 06​:24​:38 -0800, alex.jakimenko@​gmail.com wrote​:

This is probably best demonstrated by a code snippet.

Code​:

my $out = Channel.new;
#my $proc = Proc​::Async.new(‘perl6’, ‘-e’, ‘sleep ∞’); # ← this works
my $proc = Proc​::Async.new(‘perl6’, ‘-e’, ‘.say for 0..0x1FFFF’); # ←
this doesn't
$proc.stdout.tap({ $out.send​: 1 });

my $promise = $proc.start;
say ‘let's await ’, now;
await Promise.anyof(Promise.in(1), $promise);
say ‘let's close ’, now;
$proc.kill;
await $promise;
$out.close;
say ‘that's it ’, now;

The idea is that we want to kill our process if it is still working
after 1 second. This works fine, unless the process is writing a lot
of stuff to stdout.

At present, the default scheduler has a single work queue, and if the output from the process is not being processed at the rate it's coming in then the queue will end up with a backlog, and the timer event - placed into the same queue - will be delayed. In the future, we'll get around to having a smarter scheduler. In the meantime, I suggest creating a second scheduler to use for timer (or Proc​::Async) events.

/jnthn

Loading

@p6rt
Copy link
Author

@p6rt p6rt commented Dec 20, 2016

The RT System itself - Status changed from 'new' to 'open'

Loading

@p6rt
Copy link
Author

@p6rt p6rt commented Oct 5, 2017

From @jnthn

On Tue, 20 Dec 2016 05​:07​:57 -0800, jnthn@​jnthn.net wrote​:

On Sun, 18 Dec 2016 06​:24​:38 -0800, alex.jakimenko@​gmail.com wrote​:

This is probably best demonstrated by a code snippet.

Code​:

my $out = Channel.new;
#my $proc = Proc​::Async.new(‘perl6’, ‘-e’, ‘sleep ∞’); # ← this works
my $proc = Proc​::Async.new(‘perl6’, ‘-e’, ‘.say for 0..0x1FFFF’); # ←
this doesn't
$proc.stdout.tap({ $out.send​: 1 });

my $promise = $proc.start;
say ‘let's await ’, now;
await Promise.anyof(Promise.in(1), $promise);
say ‘let's close ’, now;
$proc.kill;
await $promise;
$out.close;
say ‘that's it ’, now;

The idea is that we want to kill our process if it is still working
after 1 second. This works fine, unless the process is writing a lot
of stuff to stdout.

At present, the default scheduler has a single work queue, and if the
output from the process is not being processed at the rate it's coming
in then the queue will end up with a backlog, and the timer event -
placed into the same queue - will be delayed. In the future, we'll get
around to having a smarter scheduler. In the meantime, I suggest
creating a second scheduler to use for timer (or Proc​::Async) events.

The new scheduler has a separate queue for time-based events, and seems to do notably better at this.

Loading

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

Successfully merging a pull request may close this issue.

None yet
1 participant