jpaulm / jsfbp

FBP implementation written using JavaScript and node-fibers
MIT License
119 stars 23 forks source link

randdelay doesn't actually delay #12

Closed tlrobinson closed 9 years ago

tlrobinson commented 9 years ago

setTimeout is an asynchronous operation (the passed in callback gets called after the delay), so this doesn't actually delay anything: https://github.com/jpaulm/jsfbp/blob/master/script/randdelay.js#L24-L28

I'm not sure what the correct implementation should be. I tried something like this https://github.com/laverdet/node-fibers#sleep which seemed to correctly delay, but then the process never exits.

jpaulm commented 9 years ago

@tlrobinson I guess I don't see why not! If you run fbptest11, you will see IPs coming out of the final merge in a somewhat shuffled order, just as I intended. Or are you saying the name isn't really accurate - that may be true! Feel free to suggest a better one...

Thanks,

Paul

On Wed, Feb 11, 2015 at 4:07 PM, Tom Robinson notifications@github.com wrote:

setTimeout is an asynchronous operation (the passed in callback gets called after the delay), so this doesn't actually delay anything: https://github.com/jpaulm/jsfbp/blob/master/script/randdelay.js#L24-L28

I'm not sure what the correct implementation should be. I tried something like this https://github.com/laverdet/node-fibers#sleep which seemed to correctly delay, but then the process never exits.

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12.

tlrobinson commented 9 years ago

Calls to setTimeout don't work with fibers. setTimeout(function() {}, whatever) might as well be removed since it does absolutely nothing. Here's an equivalent (non-random) delay component and test case:

function delay() {
  var delayPort = fbp.InputPort.openInputPort('DELAY');
  var inPort = fbp.InputPort.openInputPort('IN');
  var outPort = fbp.OutputPort.openOutputPort('OUT');

  var delayP = delayPort.receive();
  fbp.IP.drop(delayP);
  var delay = parseFloat(delayP.contents);

  var p;
  while (p = inPort.receive()) {
    sleep(delay);
    outPort.send(p);
  }
}

function sleep(ms) {
  return setTimeout(function() {}, ms);
}

describe('copier', function() {
  it('should copy input to output', function(){
    var result = [];

    var senderA = fbp.defProc(MockSender(['100 (a)', '200 (a)', '300 (a)', '400 (a)']));
    var senderB = fbp.defProc(MockSender(['250 (b)', '500 (b)', '750 (b)', '1000 (b)']));
    var delayA = fbp.defProc(delay);
    var delayB = fbp.defProc(delay);
    var recvr  = fbp.defProc(MockReceiver(result));

    fbp.initialize(delayA, 'DELAY', 100);
    fbp.initialize(delayB, 'DELAY', 250);
    fbp.connect(senderA, 'OUT', delayA, 'IN', 5);
    fbp.connect(senderB, 'OUT', delayB, 'IN', 5);
    fbp.connect(delayA, 'OUT', recvr, 'IN', 5);
    fbp.connect(delayB, 'OUT', recvr, 'IN', 5);

    try {
      fbp.run({ trace: false });
    } catch (e) {
      console.log(e);
    }

    expect(result).to.deep.equal(['100 (a)', '200 (a)', '250 (b)', '300 (a)', '400 (a)', '500 (b)', '750 (b)', '1000 (b)']);
  });
});

Would you agree that if the two senders are supposed to be delayed 100ms and 250ms then those IPs should arrive in that order?

The actual order is the following:

         "100 (a)"
         "200 (a)"
         "300 (a)"
         "400 (a)"
         "250 (b)"
         "500 (b)"
         "750 (b)"
         "1000 (b)"

(Also I'm getting "deadlock detected", but I'm pretty sure that's a different issue)

jpaulm commented 9 years ago

On Thu, Feb 12, 2015 at 11:12 PM, Tom Robinson notifications@github.com wrote:

Calls to setTimeout don't work with fibers. setTimeout(function() {}, whatever) might as well be removed since it does absolutely nothing. Here's an equivalent (non-random) delay component and test case:

...

function sleep(ms) { return setTimeout(function() {}, ms); }

Sorry, Tom, I'm a bit confused - you say setTimeOut(...) can be removed, but I see it in your code. I'm afraid you lost me here!

Also, in general in FBP when two output ports are connected to one input port, you can't guarantee the order of arrival of the IPs. Although I suppose in this case the delays are long enough that practically your desired order could be preserved... However, it looks to me as if the a's and the b's should be interleaved - have you actually run this yet?

Regards,

Paul

tlrobinson commented 9 years ago

Yes, I ran it, and the results are not interleaved.

I'm saying that call to setTimeout doesn't do anything (it calls that empty function after a certain amount of time, but the setTimeout function itself returns immediately) thus removing it changes nothing, but it would still be broken, of course. My test case above shows that it is broken, but I'm not sure how to fix it (I tried replacing it with the implementation of "sleep" from the node-fibers page but it doesn't work: https://github.com/laverdet/node-fibers#sleep)

jpaulm commented 9 years ago

You were right - I picked up Marcel's sleep and added it into randdelay with some small changes required for JSFBP, and the delay seems to be working now. I added a test case, fbptest13, to make the function more obvious. Not surprisingly, it takes a while to run!

jpaulm commented 9 years ago

@tlrobinson Looks like it's working - can we close this? Thx

tlrobinson commented 9 years ago

I'd like to get my test case working, which I haven't been able to, even with an updated sleep implementation. I've posted it to a branch called "delay": https://github.com/tlrobinson/jsfbp/tree/delay

You should be able to get it by doing git fetch tlrobinson and git checkout tlrobinson/delay, then npm test should result in the test failing. The test is located at test/components/delay.js

jpaulm commented 9 years ago

Thanks, will look at it tomorrow!

P.

On Fri, Feb 13, 2015 at 9:29 PM, Tom Robinson notifications@github.com wrote:

I'd like to get my test case working, which I haven't been able to, even with an updated sleep implementation. I've posted it to a branch called "delay": https://github.com/tlrobinson/jsfbp/tree/delay

You should be able to get it by doing git fetch tlrobinson and git checkout tlrobinson/delay.

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-74357691.

jpaulm commented 9 years ago

Sorry, @tlrobinson , git fetch tlrobinson didn't work - says tlrobinson is not a git repository...

On Fri, Feb 13, 2015 at 9:29 PM, Tom Robinson notifications@github.com wrote:

I'd like to get my test case working, which I haven't been able to, even with an updated sleep implementation. I've posted it to a branch called "delay": https://github.com/tlrobinson/jsfbp/tree/delay

You should be able to get it by doing git fetch tlrobinson and git checkout tlrobinson/delay.

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-74357691.

ComFreek commented 9 years ago

You must first add tlrobinson's fork as a remote repository:

git remote add tlrobinson https://github.com/tlrobinson/jsfbp.git

Then do (as the comment of @tlrobinson explains):

git fetch tlrobinson
git checkout tlrobinson/delay
npm test
jpaulm commented 9 years ago

Thanks @comfreek - I can't really imagine how @tlrobinson expects to get a predictable sequence out of this... Anyway I'll take a look this afternoon.

Regards,

Paul


On Sat, Feb 14, 2015 at 10:49 AM, ComFreek <notifications@github.com> wrote:

> You must first add tlrobinson's fork as a remote repository:
>
> git remote add tlrobinson https://github.com/tlrobinson/jsfbp.git
>
> Then do (as the comment of @tlrobinson <https://github.com/tlrobinson>
> explains):
>
> git fetch tlrobinson
> git checkout tlrobinson/delay
> npm test
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/jpaulm/jsfbp/issues/12#issuecomment-74379994>.
>
jpaulm commented 9 years ago

Thanks, @comfreek. I did a git status, and got:

 C:\Users\Paul\Documents\GitHub\fbp-repos\jsfbp [(7cd38e0...) +0 ~2 -0]>
git stat
us
HEAD detached at tlrobinson/delay

What does this mean? And can I just delete @tlrobinson's delay file when I'm finished with it?

Thanks,

Paul

On Sat, Feb 14, 2015 at 10:49 AM, ComFreek notifications@github.com wrote:

You must first add tlrobinson's fork as a remote repository:

git remote add tlrobinson https://github.com/tlrobinson/jsfbp.git

Then do (as the comment of @tlrobinson https://github.com/tlrobinson explains):

git fetch tlrobinson git checkout tlrobinson/delay npm test

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-74379994.

ComFreek commented 9 years ago

I am not a Git pro, but I suppose you're now on the tlrobinson/delay branch (due to the checkout command I mentioned in my comment). What would you like to do? If you would like to switch to your normal branch again, just issue git checkout master.

jpaulm commented 9 years ago

Yup, found that on Internet! And that also got rid of Tom's delay routine. Thanks!

On Sat, Feb 14, 2015 at 2:58 PM, ComFreek notifications@github.com wrote:

I am not a Git pro, but I suppose you're now on the tlrobinson/delay branch (due to the checkout command I mentioned in my comment). What would you like to do? If you would like to switch to your normal branch again, just issue git checkout master.

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-74389583.

jpaulm commented 9 years ago

Hi @tlrobinson , I'm still not clear on how npm test works, but I have added a fixed interval routine called delay to components, and a test for it called fbptest14 - and they both seem to work fine. Give fbptest14 a try! I have tracing turned off, to make things easier to see - not sure it does though!

Generally, though, if you have >1 arcs converging into a single input port, I wouldn't expect predictable sequences to be generated - that's anti-FBP! Although, maybe in this case...(?)

@comfreek , thanks for your help with Git!

jpaulm commented 9 years ago

@tlrobinson - what happened with this issue?

tlrobinson commented 9 years ago

I've been gone all weekend so I'll have to check it out later.

If the delay isn't random (it isn't in my test case) then I would expect the order to be relatively predictable, as long as the delays are long enough (100ms should be fine most of the time). I'm not sure why that's "anti-FBP". I understand if two IPs arrive at an input port "close" to the same time the order is unpredictable, but 100ms is an eternity for a computer. On Mon, Feb 16, 2015 at 11:27 AM jpaulm notifications@github.com wrote:

@tlrobinson https://github.com/tlrobinson - what happened with this issue?

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-74560130.

jpaulm commented 9 years ago

I agree! You had said earlier that it didn't work for you. I'm curious if it is now working...

Regards,

Paul

On Mon, Feb 16, 2015 at 8:52 PM, Tom Robinson notifications@github.com wrote:

I've been gone all weekend so I'll have to check it out later.

If the delay isn't random (it isn't in my test case) then I would expect the order to be relatively predictable, as long as the delays are long enough (100ms should be fine most of the time). I'm not sure why that's "anti-FBP". I understand if two IPs arrive at an input port "close" to the same time the order is unpredictable, but 100ms is an eternity for a computer. On Mon, Feb 16, 2015 at 11:27 AM jpaulm notifications@github.com wrote:

@tlrobinson https://github.com/tlrobinson - what happened with this issue?

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-74560130.

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-74605616.

tlrobinson commented 9 years ago

I ran fbptest14.js and it looks like it works, however fbp.run returns before all of the processes exit, which I guess is expected because it wraps run2 in a Fiber. If call run2 directly (add exports.run2 = run2 to index.js) instead then my test almost works, but some IPs are out of order (even if we increase the timeouts so there is no possibility of race conditions)

npm test just runs whatever test command we have specified in the package.json "scripts" "test" option, which we have setup to use the "mocha" unit test framework. Alternatively you can run mocha directly. Copy the following test case to "test/components/delay.js" then run "mocha --ui mocha-fibers --require test/test_helper.js test/components/delay.js" (this runs mocha with the mocha-fibers plugin and loads our "test_helper.js" file before loading the "delay.js" tests)

var fbp = require('../..');

describe('delay', function() {
  it('should delay correct amounts', function(){
    var result = [];

    var senderA = fbp.defProc(MockSender(['100 (a)', '200 (a)', '300 (a)', '400 (a)']));
    var senderB = fbp.defProc(MockSender(['250 (b)', '500 (b)', '750 (b)', '1000 (b)']));
    var delayA = fbp.defProc('./components/delay', 'delayA');
    var delayB = fbp.defProc('./components/delay', 'delayB');
    var recvr  = fbp.defProc(MockReceiver(result));

    fbp.initialize(delayA, 'INTVL', 100);
    fbp.initialize(delayB, 'INTVL', 250);
    fbp.connect(senderA, 'OUT', delayA, 'IN', 5);
    fbp.connect(senderB, 'OUT', delayB, 'IN', 5);
    fbp.connect(delayA, 'OUT', recvr, 'IN', 5);
    fbp.connect(delayB, 'OUT', recvr, 'IN', 5);

    fbp.run2({ trace: false });

    expect(result).to.deep.equal(['100 (a)', '200 (a)', '250 (b)', '300 (a)', '400 (a)', '500 (b)', '750 (b)', '1000 (b)']);
  });
});

Even if you increase the intervals to 1000 and 2500 "250 (b)" will arrive before "200 (a)" (you will probably need to add the "--no-timeouts" command line option when running mocha)

jpaulm commented 9 years ago

@tlrobinson wrote:

I ran fbptest14.js and it looks like it works, however fbp.run returns before all of the processes exit, which I guess is expected because it wraps run2 in a Fiber. If call run2 directly (add exports.run2 = run2 to index.js) instead then my test almost works, but some IPs are out of order (even if we increase the timeouts so there is no possibility of race conditions)

run2 is in a fiber, so I guess we could add something to hold up run until run2 finishes. I could look at this, but I will not have access to my desktop from tomorrow for about a month, so maybe one of you guys, @tlrobinson or @comfreek, could suggest a way to do this.

Regards,

Paul

On Mon, Feb 16, 2015 at 11:56 PM, Tom Robinson notifications@github.com wrote:

I ran fbptest14.js and it looks like it works, however fbp.run returns before all of the processes exit, which I guess is expected because it wraps run2 in a Fiber. If call run2 directly (add exports.run2 = run2 to index.js) instead then my test almost works, but some IPs are out of order (even if we increase the timeouts so there is no possibility of race conditions)

npm test just runs whatever test command we have specified in the package.json "scripts" "test" option, which we have setup to use the "mocha" unit test framework. Alternatively you can run mocha directly. Copy the following test case to "test/components/delay.js" then run "mocha --ui mocha-fibers --require test/test_helper.js test/components/delay.js" (this runs mocha with the mocha-fibers plugin and loads our "test_helper.js" file before loading the "delay.js" tests)

var fbp = require('../..');

describe('delay', function() { it('should delay correct amounts', function(){ var result = [];

var senderA = fbp.defProc(MockSender(['100 (a)', '200 (a)', '300 (a)', '400 (a)']));
var senderB = fbp.defProc(MockSender(['250 (b)', '500 (b)', '750 (b)', '1000 (b)']));
var delayA = fbp.defProc('./components/delay', 'delayA');
var delayB = fbp.defProc('./components/delay', 'delayB');
var recvr  = fbp.defProc(MockReceiver(result));

fbp.initialize(delayA, 'INTVL', 100);
fbp.initialize(delayB, 'INTVL', 250);
fbp.connect(senderA, 'OUT', delayA, 'IN', 5);
fbp.connect(senderB, 'OUT', delayB, 'IN', 5);
fbp.connect(delayA, 'OUT', recvr, 'IN', 5);
fbp.connect(delayB, 'OUT', recvr, 'IN', 5);

fbp.run2({ trace: false });

expect(result).to.deep.equal(['100 (a)', '200 (a)', '250 (b)', '300 (a)', '400 (a)', '500 (b)', '750 (b)', '1000 (b)']);

}); });

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-74617177.

ComFreek commented 9 years ago

I found waitfor. Especially waitfor.js#56 is of interest: https://github.com/luciotato/waitfor/blob/master/waitfor.js#L56

ComFreek commented 9 years ago

waitfor does only work in a Fiber itself, so we would have to put run() into a Fiber. But then again the caller of run() would need to put his code into the same Fiber to preserve synchronicity. Eventually, you get to a point which is asynchronous!

One could use deasync which intervenes at the Node.js C++ level, but that introduces another (binary) dependency.

Why don't we just stick to the asynchronicity and callbacks?

I've modified index.js and fbptest14.js to use callbacks:

// index.js
function run(options, callback) {
  options = options || {};

  Fiber(function() {
    run2(options.trace);
    callback(null);
  }).run();
}

// fbptest14.js
fbp.run(null, function () {
  console.log("FINISHED!");
});

Output:

[...]
delay0 start sleep: 2000 msecs
data: to form different applications without having to be changed internally. FBP is
data: thus naturally component-oriented.
Elapsed time in secs: 14.657
FINISHED!
jpaulm commented 9 years ago

Hi @comfreek 1) how did you guys know that run finished before run2 finished? 2) does it matter?!

I can put your suggested change into index.js and the examples, but I'm just curious why it's necessary...

Regards,

Paul

As I've said elsewhere, I won't be able to touch the code for a month, but I'm sure your forks will be very interesting by the time I get back! :-)

ComFreek commented 9 years ago

1) how did you guys know that run finished before run2 finished?

I asked myself the same question :). It's verifiable using one line:

// fbptest14.js
[...]
fbp.run();
console.log("FINISHED");

The output will be as follows:

[...]
FINISHED
[...]
Elapsed time in secs: xyz

(The last line is from the end of the function run2().)

In my view, having a callback function – which is run when the network has terminated – is essential because there are numerous cases where one (=user of JSFBP) wants to do something after the network has truly terminated. Without the change there is no real/clean way to tell when the network has finished. (One hacky way would be to create a component whose sole reason is to call a function upon receiving an IP.)


(/cc @tlrobinson) I am currently splitting index.js again, so that we eventually have the following files:

What do you think?

tlrobinson commented 9 years ago

Determining when the network has completed is a secondary concern to me... the fact that some of the IPs appear out of order is more puzzling. Are either of you able to reproduce that issues?

i.e. this comment of mine:

then my test almost works, but some IPs are out of order

and

Even if you increase the intervals to 1000 and 2500 "250 (b)" will arrive before "200 (a)"

jpaulm commented 9 years ago

@comfreek - is your split ready for merging? I will have a little time tomorrow - and then that's it for a while!

@tlrobinson - re your IPs arriving out of sequence, I think you are forgetting about possible delays in the connections. IPs are sent when there is room for them in a connection, and received at a fairly random time after that, depending on what is ready to be processed in the "future events queue". FBP doesn't guarantee any particular sequence of processing, as long as the 2 constraints I describe in my book are satisfied. I think we will have to stipulate that the "expected results" thing in your test environment will only work when there is no many-to-one merging going on (anywhere in the network)... unless, of course, you sort the output before doing the check (we actually did that for the trace output of one of our on-line systems).

Regards,

Paul

tlrobinson commented 9 years ago

@jpaulm Why would one IP be delayed 500ms more than another IP within the same OS process? Possible if these IPs were being transmitted over the network around the world, but not within the same process on the same machine.

ComFreek commented 9 years ago

@jpaulm No, I'm afraid most of the examples are still not working.

jpaulm commented 9 years ago

I cooked up a test, but it conflicts on delay.js and fbptest14 - and I don't have time to fix the conflicts (heading to the airport in a little while!)

Here are the two components:


'use strict';

var InputPort = require('../core/InputPort')
  , OutputPort = require('../core/OutputPort')
  , Fiber = require('fibers')
  , IP = require('../core/IP');

module.exports = function delay(runtime) {
  var proc = runtime.getCurrentProc();
  var inport = InputPort.openInputPort('IN');
  var intvlport = InputPort.openInputPort('INTVL');
  var outport = OutputPort.openOutputPort('OUT');
  var intvl_ip = intvlport.receive();
  var intvl = intvl_ip.contents;
  IP.drop(intvl_ip);

  while (true) {
    var ip = inport.receive();
    if (ip === null) {
      break;
    }
<<<<<<< HEAD
    fbp.setCallbackPending(true);
    console.log('start wait for ' + Math.round(intvl * 100) / 100 + ' msecs: ' + ip.contents);
    sleep(proc, intvl);
    fbp.setCallbackPending(false);
=======
    runtime.setCallbackPending(true);
    sleep(runtime, proc, intvl);
    runtime.setCallbackPending(false);
>>>>>>> 622568d17a5d83b1cd2fb4d06e9b8b5381ce91dd
    outport.send(ip);
  }
} 

<<<<<<< HEAD
function sleep(proc, ms) {
    // console.log(proc.name + ' start sleep: ' + Math.round(ms * 100) / 100 + ' msecs');  
    var fiber = Fiber.current;
    setTimeout(function() {
        console.log('end wait for ' + Math.round(ms * 100) / 100 + ' msecs');
        fbp.queueCallback(proc);
=======
function sleep(runtime, proc, ms) {
  console.log(proc.name + ' start sleep: ' + Math.round(ms * 100) / 100 + ' msecs');  
    var fiber = Fiber.current;
    setTimeout(function() {
        runtime.queueCallback(proc);
>>>>>>> 622568d17a5d83b1cd2fb4d06e9b8b5381ce91dd
    }, ms);
    return Fiber.yield();
}

'use strict';

var fbp = require('..')
  , InputPort = require('../core/InputPort')
  , IP = require('../core/IP')
  , OutputPort = require('../core/OutputPort');

module.exports = function mockmocksender() {
  var inport = InputPort.openInputPort('PARMS');
  var outport = OutputPort.openOutputPort('OUT');
  var ip = inport.receive();
  var parms = ip.contents;
  var parmsarray = parms.split(',');  // increment, suffix, count
  IP.drop(ip);
  //console.log(count);
  var value = 0;
  for (var i = 0; i < parmsarray[2]; i++) {
    value += Number(parmsarray[0]);
    var ip = IP.create(value + ' ' + parmsarray[1] );
    if (-1 == outport.send(ip)) {
      return;
    }
  }
};

and the test case


var fbp = require('..');

// --- define network ---
<<<<<<< HEAD
//var mms0     = fbp.defProc('./components/mockmocksender', 'mms0');
//var delay0      = fbp.defProc('./components/delay', 'delay0');
var mms1     = fbp.defProc('./components/mockmocksender', 'mms1');
var delay1      = fbp.defProc('./components/delay', 'delay1');
var recvr      = fbp.defProc('./components/recvr');

//fbp.initialize(delay0, 'INTVL', '100');   // 100 msecs
fbp.initialize(delay1, 'INTVL', '250');   // 250 msecs
//fbp.initialize(mms0, 'PARMS', '100,(a),4');
//fbp.connect(mms0, 'OUT', delay0, 'IN', 5);
fbp.initialize(mms1, 'PARMS', '250,(b),4');
fbp.connect(mms1, 'OUT', delay1, 'IN', 5);
//fbp.connect(delay0, 'OUT', recvr, 'IN', 5);
fbp.connect(delay1, 'OUT', recvr, 'IN', 5);
               // --- run ---
fbp.run({ trace: true });
//fbp.run();

Maybe one of you can figure out what is wrong! TIA

jpaulm commented 9 years ago

@ComFreek if you can figure out my test, you will see that one delay somehow caught up with the next! It seems that the delay is happening, or at least most of them, but not in the right place. The odd thing is that what Msrcel shows is almost the same code. Should we raise an issue on node-fibers?

ComFreek commented 9 years ago

Hope you had a pleasant flight!

The test case in your comment above includes a 'mockmocksender' component. Could you please share the code of it? I cannot find it inside this repository.

I am afraid I do not know Marcel.

PS: I don't know where your merge conflicts come from. My fork is now even with this repository, meaning they have equivalent code bases.

jpaulm commented 9 years ago

Thanks, @comfreek , a bit bumpy for a while, but most of it was fine! Temperature here is usually in the high 20s!

The problem is that Github is very hard to use here (small tablet screen, shaky Wifi, etc.), so I put everything in my previous post, and hoped you will be able to figure things out!

In my previous post there are two routines: delay and mockmocksender (Tom's mocksender convrrted to a regular JSFBP component). Also fbptest14 modified to just have one reader and one delay instance. The delay I tried to put up is basically the same as the one on Github, except for some changes to the logging.

So they are not real conflicts - just me trying to put some useful stuff on Github in a hurry!

Marcel is Marcel Laverdet, author of node-fibers. But on second thoughts, it's still probably some odd interaction between timeout and my future events logic! Reader and Writer callbacks seem to work, and Marcel thinks his code works, so the problem should be tractable!

Thanks and all the best,

Paul

Hope you had a pleasant flight!

The test case in your comment above includes a 'mockmocksender' component. Could you please share the code of it? I cannot find it inside this repository.

I am afraid I do not know Marcel.

PS: I don't know where your merge conflicts come from. My fork is now even with this repository, meaning they have equivalent code bases.

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-75266149.

ComFreek commented 9 years ago

On my system the IPs arrive in the right order at recvr, please see the output below. I have saved the test case from your comment above into P:\test.js.

P:\jsfbp>node ./test.js
Start time: 2015-02-22T12:19:19.682Z
creating new fiber for mms1
mms1 fiber started
mms1 recv IIP from port mms1.PARMS: 250,(b),4
mms1 Create IP with: 250,(b),4
mms1 IP dropped with: 250,(b),4
mms1 Create IP with: 250 (b)
mms1 send to mms1.OUT: 250 (b)
mms1 send OK
mms1 Create IP with: 500 (b)
mms1 send to mms1.OUT: 500 (b)
mms1 send OK
mms1 Create IP with: 750 (b)
mms1 send to mms1.OUT: 750 (b)
mms1 send OK
mms1 Create IP with: 1000 (b)
mms1 send to mms1.OUT: 1000 (b)
mms1 send OK
mms1 yielded: false, cbpending: false
mms1 fiber ended
mms1 closing
mms1 closed
creating new fiber for delay1
delay1 fiber started
delay1 recv IIP from port delay1.INTVL: 250
delay1 Create IP with: 250
delay1 IP dropped with: 250
delay1 recv from delay1.IN
delay1 recv OK: 250 (b)
delay1 start sleep: 250 msecs
delay1 yielded: false, cbpending: true
delay1 fiber awaiting callback
creating new fiber for recvr
recvr fiber started
recvr recv from recvr.IN
recvr yielded: true, cbpending: false
recvr fiber yielded
delay1 fiber callback run
delay1 send to delay1.OUT: 250 (b)
delay1 send OK
delay1 recv from delay1.IN
delay1 recv OK: 500 (b)
delay1 start sleep: 250 msecs
delay1 yielded: false, cbpending: true
delay1 fiber awaiting callback
recvr fiber resumed
recvr recv OK: 250 (b)
data: 250 (b)
recvr IP dropped with: 250 (b)
recvr recv from recvr.IN
recvr yielded: true, cbpending: false
recvr fiber yielded
queue delay1
queue delay1
delay1 fiber callback run
delay1 send to delay1.OUT: 500 (b)
delay1 send OK
delay1 recv from delay1.IN
delay1 recv OK: 750 (b)
delay1 start sleep: 250 msecs
delay1 yielded: false, cbpending: true
delay1 fiber awaiting callback
delay1 fiber callback run
delay1 send to delay1.OUT: 750 (b)
delay1 send OK
delay1 recv from delay1.IN
delay1 recv OK: 1000 (b)
delay1 start sleep: 250 msecs
delay1 yielded: false, cbpending: true
delay1 fiber awaiting callback
recvr fiber resumed
recvr recv OK: 500 (b)
data: 500 (b)
recvr IP dropped with: 500 (b)
recvr recv from recvr.IN
recvr recv OK: 750 (b)
data: 750 (b)
recvr IP dropped with: 750 (b)
recvr recv from recvr.IN
recvr yielded: true, cbpending: false
recvr fiber yielded
queue delay1
queue delay1
delay1 fiber callback run
delay1 send to delay1.OUT: 1000 (b)
delay1 send OK
delay1 recv from delay1.IN
delay1 recv EOS from delay1.IN
delay1 yielded: false, cbpending: false
delay1 fiber ended
delay1 closing
delay1 closed
recvr fiber resumed
recvr recv OK: 1000 (b)
data: 1000 (b)
recvr IP dropped with: 1000 (b)
recvr recv from recvr.IN
recvr recv EOS from recvr.IN
recvr yielded: false, cbpending: false
recvr fiber ended
recvr closing
recvr closed
Elapsed time in secs: 0.687
jpaulm commented 9 years ago

Odd! In my copy of 'delay' I added a log line when the timeout completes, and I don't see that. If you add that, IIRC 2 messages saying 250 ms came out in quick succession, with no events in between. But I didn't have time to investigate further.

Of course I can't access my testbed for a month, so I'm thinking we should just drop it until I get back - unless someone gets a brainwave! :-)

@kenhkan does fbptest14 run on your new system?

Regards,

Paul

On Feb 22, 2015 7:38 AM, "ComFreek" notifications@github.com wrote:

On my system the IPs arrive in the right order at recvr, please see the output below. I have saved the test case from your comment above into P:\test.js.

P:\jsfbp>node ./test.js Start time: 2015-02-22T12:19:19.682Z creating new fiber for mms1 mms1 fiber started mms1 recv IIP from port mms1.PARMS: 250,(b),4 mms1 Create IP with: 250,(b),4 mms1 IP dropped with: 250,(b),4 mms1 Create IP with: 250 (b) mms1 send to mms1.OUT: 250 (b) mms1 send OK mms1 Create IP with: 500 (b) mms1 send to mms1.OUT: 500 (b) mms1 send OK mms1 Create IP with: 750 (b) mms1 send to mms1.OUT: 750 (b) mms1 send OK mms1 Create IP with: 1000 (b) mms1 send to mms1.OUT: 1000 (b) mms1 send OK mms1 yielded: false, cbpending: false mms1 fiber ended mms1 closing mms1 closed creating new fiber for delay1 delay1 fiber started delay1 recv IIP from port delay1.INTVL: 250 delay1 Create IP with: 250 delay1 IP dropped with: 250 delay1 recv from delay1.IN delay1 recv OK: 250 (b) delay1 start sleep: 250 msecs delay1 yielded: false, cbpending: true delay1 fiber awaiting callback creating new fiber for recvr recvr fiber started recvr recv from recvr.IN recvr yielded: true, cbpending: false recvr fiber yielded delay1 fiber callback run delay1 send to delay1.OUT: 250 (b) delay1 send OK delay1 recv from delay1.IN delay1 recv OK: 500 (b) delay1 start sleep: 250 msecs delay1 yielded: false, cbpending: true delay1 fiber awaiting callback recvr fiber resumed recvr recv OK: 250 (b) data: 250 (b) recvr IP dropped with: 250 (b) recvr recv from recvr.IN recvr yielded: true, cbpending: false recvr fiber yielded queue delay1 queue delay1 delay1 fiber callback run delay1 send to delay1.OUT: 500 (b) delay1 send OK delay1 recv from delay1.IN delay1 recv OK: 750 (b) delay1 start sleep: 250 msecs delay1 yielded: false, cbpending: true delay1 fiber awaiting callback delay1 fiber callback run delay1 send to delay1.OUT: 750 (b) delay1 send OK delay1 recv from delay1.IN delay1 recv OK: 1000 (b) delay1 start sleep: 250 msecs delay1 yielded: false, cbpending: true delay1 fiber awaiting callback recvr fiber resumed recvr recv OK: 500 (b) data: 500 (b) recvr IP dropped with: 500 (b) recvr recv from recvr.IN recvr recv OK: 750 (b) data: 750 (b) recvr IP dropped with: 750 (b) recvr recv from recvr.IN recvr yielded: true, cbpending: false recvr fiber yielded queue delay1 queue delay1 delay1 fiber callback run delay1 send to delay1.OUT: 1000 (b) delay1 send OK delay1 recv from delay1.IN delay1 recv EOS from delay1.IN delay1 yielded: false, cbpending: false delay1 fiber ended delay1 closing delay1 closed recvr fiber resumed recvr recv OK: 1000 (b) data: 1000 (b) recvr IP dropped with: 1000 (b) recvr recv from recvr.IN recvr recv EOS from recvr.IN recvr yielded: false, cbpending: false recvr fiber ended recvr closing recvr closed Elapsed time in secs: 0.687

— Reply to this email directly or view it on GitHub.

ComFreek commented 9 years ago

@jpaulm I have come across related issues now:

I added two automated tests for delay.js and randdelay.js: delay.js and randdelay.js. (Note that they're both on a separate branch called 'fix-delay'.)

Running both tests resulted in failures because the elapsed time span (which the test measured) is smaller than 1000 ± 150ms (the specified delay ± time buffer for inaccuracies and to account for the network's startup and processing time). In fact the elapsed time averages 0ms.
I have found out that the callback passed to Network#run, which gets called when the runtime has finished, gets called before the actual finish! Therefore, the elapsed time is measured as nearly 0ms.

The problem is that [any process].yielded doesn't get set when that process runs a callback as delay and randdelay do. In these cases the FiberRuntime seems to quit working as it thinks that all work is done. Therefore, we must ensure that [delay process].yielded is set to true before Fiber.yield() gets called (l. 37 in delay.js) and that it is set to false when the callback has run, i.e. just before the process gets added to the queue of processes again (l. 34 in delay.js).

All examples seem to run fine. I will try to run Tom's test later.

Finally, I think that correctly setting proc.yielded is not an exercise which a process should do. This should be task of the runtime. However, this is just a stylistic problem which can be fixed anytime

jpaulm commented 9 years ago

Many thanks - it sounds like you have solved the problem!

Could you make the required changes, and retest? TIA

By the way, does this problem apply to reader and writer? It sounds like it might...

Regards,

Paul On Feb 23, 2015 9:20 AM, "ComFreek" notifications@github.com wrote:

@jpaulm https://github.com/jpaulm I have come across related issues now:

I added two automated tests for delay.js and randdelay.js: delay.js https://github.com/jpaulm/jsfbp/blob/fix-delay/test/components/delay.js and randdelay.js https://github.com/jpaulm/jsfbp/blob/fix-delay/test/components/randdelay.js. (Note that they're both on a separate branch called 'fix-delay'.)

Running both tests resulted in failures because the elapsed time span (which the test measured) is smaller than 1000 ± 150ms (the specified delay ± time buffer for inaccuracies and to account for the network's startup and processing time). In fact the elapsed time averages 0ms.

I have found out that the callback passed to Network#run, which gets called when the runtime has finished, gets called before the actual finish! Therefore, the elapsed time is measured as nearly 0ms.

The problem is that [any process].yielded doesn't get set when that process runs a callback as delay and randdelay do. In these cases the FiberRuntime seems to quit working as it thinks that all work is done. Therefore, we must ensure that [delay process].yielded is set to true before Fiber.yield() gets called (l. 37 in delay.js https://github.com/jpaulm/jsfbp/blob/2912b13b2e967bb8b93ef5d6ed426ce2776c2a5e/components/delay.js#L37) and that it is set to false when the callback has run, i.e. just before the process gets added to the queue of processes again (l. 34 in delay.js https://github.com/jpaulm/jsfbp/blob/2912b13b2e967bb8b93ef5d6ed426ce2776c2a5e/components/delay.js#L34 ).

All examples seem to run fine. I will try to run Tom's test https://github.com/tlrobinson/jsfbp/blob/delay/test/components/delay.js later.

Finally, I think that correctly setting proc.yielded is not an exercise which a process should do. This should be task of the runtime. However, this is just a stylistic problem which can be fixed anytime

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-75548055.

ComFreek commented 9 years ago

In the course of re-testing multiple times to ensure the absence of errors I found out that errors sometimes still appear and sometimes not. I suspect a race condition. Maybe turning off tracing/console.log statements play a role.

You're right: Theoretically, the problem could also appear in reader and writer. But why do the examples using these components work? Depending on the actual bug (whose cause we haven't found yet) there might be circumstances (additional logging or components, ...) which suppress the bug.

I would like to further investigate the central run method and running scheduler (link to its code block in core/runtimes/FiberRuntime/index.js). Do you have a schematic (possibly language-agnostic) version of the algorithm? It would ease proofreading the actual implementation.

Regards, ComFreek

kenhkan commented 9 years ago

@jpaulm Example 14 runs fine with the experimental runtime.

@ComFreek @jpaulm Sorry that I fail to see where the issue is, even having read through the thread several times. Is the race condition at the runtime level? It would be odd otherwise because I thought FBP was supposed to remove race conditions by embracing processes being asynchronous at the graph level.. Some pointers to what the alleged problem would be appreciated. :)

jpaulm commented 9 years ago

@kenhkan That's interesting, and good news! The problem for us only seems to come up with delay and randdelay components, so - a) are you using setTimeout, and/or b) have you noticed any unexpected timings using it?

@comfreek Ken's result makes me think that the problem is with setTimeout, esp. as reader and writer seem to work. The problem feels to me as if the setTimeout is not working reliably in the node fibers environment, even though Marcel Laverdet says it is.

@comfreek there is not much to the scheduler logic - apart from initialization it basically just does a run (line 148) on whatever is at the top of queue. I could try to work up some pseudocode but it might take a few days, because of lack of time! Not sure how useful you would find it, though!

Regards,

Paul On Feb 24, 2015 3:14 AM, "Kenneth Kan" notifications@github.com wrote:

@jpaulm https://github.com/jpaulm Example 14 runs fine with the experimental runtime.

@ComFreek https://github.com/ComFreek @jpaulm https://github.com/jpaulm Sorry that I fail to see where the issue is, even having read through the thread several times. Is the race condition at the runtime level? It would be odd otherwise because I thought FBP was supposed to remove race conditions by embracing processes being asynchronous at the graph level.. Some pointers to what the alleged problem would be appreciated. :)

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-75715391.

ComFreek commented 9 years ago

@jpaulm @kenhkan Here's how you can reproduce the problem:

  1. git clone https://github.com/jpaulm/jsfbp.git (or update your clone with git fetch origin)
  2. git checkout fix-delay
  3. npm install
  4. Run ./node_modules/.bin/mocha --ui mocha-fibers --require test/test_helper.js test/components/randdelay.js

    (You have to use the following code when using cmd.exe (and not PowerShell): "./node_modules/.bin/Mocha" --ui mocha-fibers --require test/test_helper.js test/components/randdelay.js.)

    Run the test command multiple times. It will sometimes fail and sometimes succeed.

  5. Increase the timeout in test/components/randdelay.js#36. Replace the line by this.timeout(10000) for example. You will now see that multiple executions of the program lead to vastly different elapsed times (as reported by Mocha).

Wait a moment! It may only be a an error in reasoning. How much does/should randdelay delay? In my opinion it should maximally (as in during the whole network execution) delay the amount of milliseconds specified in its INTVL port. However, the current implementation only allows "cumulative delaying" (for lack of a better term): The second IP is processed (=delayed) only after the delay of the first IP has already been elapsed.

Example network: 1,2,3 ---> [randdelay] ---> [recvr]

Non-cumulative delaying: Maximum delay in total = value specified for the INTVL port

Cumulative delaying: Maximum delay in total = (value specified for the INTVL port) * number of IPs. Let's say that we chose 1000ms for the INTVL port and that randdelay would always come up with 1000ms as the "random value" (=we assume the worst case), then the chain of actions would be as follows:

[1000ms delay] --> '1' is sent --> [1000ms delay] --> '2' is sent --> [1000ms delay] --> '3' is sent --> [END]

If this thesis were true, then the total run time of the network would equal the sum of all randomly chosen delays, wouldn't it? Consider the following output of the test command given above:

  randdelay
Start time: 2015-02-24T17:17:49.796Z
randdelay start sleep: 607.22 msecs
Elapsed time in secs: 0.646
    V should randomly delay a single IP (651ms)
Start time: 2015-02-24T17:17:50.448Z
randdelay start sleep: 204.8 msecs
randdelay start sleep: 846.03 msecs
randdelay start sleep: 137.98 msecs
randdelay start sleep: 803.55 msecs
randdelay start sleep: 804.06 msecs
Elapsed time in secs: 1.307
    V should randomly delay multiple IPs (1309ms)

  2 passing (2s)

Regarding the second test only, we can clearly see that 204.8+846.03+...+804.06 don't add up to 1307ms as reported by the test runner Mocha. The delays can't possibly overlap, can they? randdelay.js only starts a new delay if the previous IP has been sent.

@jpaulm No worries! I just thought that I could save some time in case you had already made one.

kenhkan commented 9 years ago

@jpaulm If I read the example files correctly, example 14 is supposed to just send the second file first, right? Example 11 and 13 are supposed to be the ones exhibiting random behaviors? If that's the case, it seems to be correct, but just in case, does this output make sense to you?

@jpaulm Everything seems to be predictable. And no, I'm not using setTimeout, which is really inefficient. The loop is basically manually managed by the runtime. It gets invoked at most once per event loop (on nodejs with nextTick) and at most once per frame (in browsers with requestAnimationFrame).

@ComFreek I actually thought that it was the intended behavior. Should it work that way? randdelay.js seems to work under the "non-cumulative" model with the experimental runtime; I'm guessing Fibers is stacking the threads. (I don't know too much about the internals but I'm pretty sure it's not the semantics of the component.)

jpaulm commented 9 years ago

@kenhkan Example 14 was supposed to show simple cumulative delay behaviour. So I can't tell much from the gist. I was hoping @comfreek could simplify test 14 to just have one branch, and add some more logging

Just to be clear, 11 and 13 are supposed to show random delay behavior and 14 non-random behavior. And all 3 should be cumulative. So @comfreek 's results are definitely weird!

Best regards,

Paul On Feb 24, 2015 3:36 PM, "Kenneth Kan" notifications@github.com wrote:

@jpaulm https://github.com/jpaulm If I read the example files correctly, example 14 is supposed to just send the second file first, right? Example 11 and 13 are supposed to be the ones exhibiting random behaviors? If that's the case, it seems to be correct, but just in case, does this output https://gist.github.com/kenhkan/e42067d2a0582c44a8cf make sense to you?

@jpaulm https://github.com/jpaulm Everything seems to be predictable. And no, I'm not using setTimeout, which is really inefficient. The loop is basically manually managed by the runtime. It gets invoked at most once per event loop (on nodejs with nextTick) and at most once per frame (in browsers with requestAnimationFrame).

@ComFreek https://github.com/ComFreek I actually thought that it was the intended behavior. Should it work that way? randdelay.js seems to work under the "non-cumulative" model with the experimental runtime; I'm guessing Fibers is stacking the threads. (I don't know too much about the internals but I'm pretty sure it's not the semantics of the component.)

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-75829028.

kenhkan commented 9 years ago

Hmmm. Sorry Paul. I actually don't understand why it should be cumulative. I thought it was due to back-pressure as you set the capacity to 2 but it doesn't seem to be the reason.

More importantly, why should it be cumulative? I agree with @ComFreek that it makes sense to just send the incoming messages "immediately" given sufficient capacity. In this case "immediately" would be whatever the delay amount set by the component. Am I missing something?

jpaulm commented 9 years ago

I'm the one who is missing something! Both randdelay and delay are loops which say roughly

while not end of stream { receive sleep send }

so I don't see how it could be anything but cumulative! What am I missing?! On Feb 25, 2015 11:49 AM, "Kenneth Kan" notifications@github.com wrote:

Hmmm. Sorry Paul. I actually don't understand why it should be cumulative. I thought it was due to back-pressure as you set the capacity to 2 but it doesn't seem to be the reason.

More importantly, why should it be cumulative? I agree with @ComFreek https://github.com/ComFreek that it makes sense to just send the incoming messages "immediately" given sufficient capacity. In this case "immediately" would be whatever the delay amount set by the component. Am I missing something?

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-75984410.

kenhkan commented 9 years ago

Got it. No Paul. It's not you. It's us. It's the JS mentality. This is what happens when a language uses an asynchronous coding style.

This is actually potentially a huge issue with newcomers in the future that we need to make sure we stress somewhere. In JS, it's unintuitive to think synchronously (read: there is no sleeping!). This is also the reason behind issue #24 regarding process state and my response to @tlrobinson. In short, looper is conceptually impossible in JS, because there is no concept of a standalone process (in the general computing sense, not FBP's process). I suppose this is also the reason behind the whole loopers vs non-loopers discussion 2 years ago in the noflo community.

I actually need to update my experimental runtime to reflect this, as I completely missed it. Thanks for bringing this up @ComFreek !

kenhkan commented 9 years ago

@tlrobinson @ComFreek It's now obvious that the purpose of the while () { ... } construct in Paul's eyes (and in the FBP sense) is different than what it implies in JS semantics. We somehow need to come up with a looper syntactic construct that is intuitive in JS. Any idea?

kenhkan commented 9 years ago

Sorry guys. All resolved. See #24 . It only isn't idiomatic with es5, not es6. Looking forward to seeing your solutions with promises and streams!

jpaulm commented 9 years ago

Is it all resolved? Don't we still have a bug?!

Regards,

Paul On Feb 26, 2015 2:48 AM, "Kenneth Kan" notifications@github.com wrote:

Sorry guys. All resolved. See #24 https://github.com/jpaulm/jsfbp/issues/24 . It only isn't idiomatic with es5, not es6. Looking forward to seeing your solutions with promises and streams!

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-76130557.

jpaulm commented 9 years ago

Double check: ES6 has processes?

How do they do that without multiple stacks? It's got to be pretty complex!

Paul On Feb 26, 2015 8:41 AM, "Paul Morrison" jpaulmorr@gmail.com wrote:

Is it all resolved? Don't we still have a bug?!

Regards,

Paul On Feb 26, 2015 2:48 AM, "Kenneth Kan" notifications@github.com wrote:

Sorry guys. All resolved. See #24 https://github.com/jpaulm/jsfbp/issues/24 . It only isn't idiomatic with es5, not es6. Looking forward to seeing your solutions with promises and streams!

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-76130557.

kenhkan commented 9 years ago

Sorry for the confusion. Only the problem that I was raising was resolved.

And no, es6 doesn't have processes, but with generators you can achieve the illusion of a process via a synchronous style for async operations, much like what fiber offers.

On Thursday, February 26, 2015, jpaulm notifications@github.com wrote:

Double check: ES6 has processes?

How do they do that without multiple stacks? It's got to be pretty complex!

Paul On Feb 26, 2015 8:41 AM, "Paul Morrison" <jpaulmorr@gmail.com javascript:_e(%7B%7D,'cvml','jpaulmorr@gmail.com');> wrote:

Is it all resolved? Don't we still have a bug?!

Regards,

Paul On Feb 26, 2015 2:48 AM, "Kenneth Kan" <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Sorry guys. All resolved. See #24 https://github.com/jpaulm/jsfbp/issues/24 . It only isn't idiomatic with es5, not es6. Looking forward to seeing your solutions with promises and streams!

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-76130557.

— Reply to this email directly or view it on GitHub https://github.com/jpaulm/jsfbp/issues/12#issuecomment-76178586.