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

Behaviour Of _.parallel With fork() #234

Closed
svozza opened this issue Feb 19, 2015 · 2 comments · Fixed by #302
Closed

Behaviour Of _.parallel With fork() #234

svozza opened this issue Feb 19, 2015 · 2 comments · Fixed by #302
Labels

Comments

@svozza
Copy link
Collaborator

svozza commented Feb 19, 2015

I'm seeing some strange behaviour with parallel when joining some forked streams and was just wondering if I'm missing something. Consider this trivial example:

var _ = require('highland');

function addTen(a) {
    return a + 10;
}

function addTwenty(a) {
    return a + 20;
}

var baseRange = _([0, 1, 2, 3, 4, 5, 6, 7, 8, 9]);
var loRange = baseRange.fork();
var midRange = baseRange.fork().map(addTen);
var hiRange = baseRange.fork().map(addTwenty);

_([loRange, midRange, hiRange])
    .parallel ( 3 )
    .toArray(_.log);

// Expected output:
// => [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
//  10, 11, 12, 13, 14, 15, 16, 17, 18, 19,
//  20, 21, 22, 23, 24, 25, 26, 27, 28, 29 ]

//Actual output:
=> [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9]

It seems like only the first stream is used but if I use _.merge() all the streams are read and likewise if I create separate streams rather than forking them or even if I use observe() I get all the transformed elements back.

Is this somehow related to #40 ? And also, given that _.merge() works in parallel too and in this case I don't particularly care about the order of the elements, should I just be using that?

@vqvu vqvu added the bug label Feb 19, 2015
@vqvu
Copy link
Collaborator

vqvu commented Feb 19, 2015

This is definitely a bug in parallel. All streams should be consumed, since you have a parallelism factor of three, so it's not an issue of one stream holding up the rest.

I suspect it has to do with the shift without flushing the buffer here, which only shows up with synchronous forks because of how well-coordinated the resulting streams are.

In any case, you only need to use parallel if you care about the stream order or you care about limiting the level of parallelism. Otherwise, merge is better, since it doesn't buffer anything.

@svozza
Copy link
Collaborator Author

svozza commented Feb 20, 2015

Cool, I'll switch to merge and if I get a chance over the weekend I'll look into fixing the buffer issue.

@vqvu vqvu closed this as completed in #302 May 21, 2015
vqvu added a commit that referenced this issue May 21, 2015
…arallel

Fix buffering Issue With parallel(). Fixes #234.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants