cross-platform / dspatch

The Refreshingly Simple Cross-Platform C++ Dataflow / Patching / Pipelining / Graph Processing / Stream Processing / Reactive Programming Framework
https://flowbasedprogramming.com/
BSD 2-Clause "Simplified" License
216 stars 44 forks source link

spurious auto-ticking when pipeline depth is greater than one but ticking manually #47

Closed axjollivier closed 1 year ago

axjollivier commented 1 year ago

Hi,

When evaluating dspatch I've noticed some behaviour which is not correct. The basic setup is as follows:

// create a circuit with 3 "no-op" components connected in series
// these "no-op" test components simply count how many times they have been ticked
auto circuit = std::make_shared<DSPatch::Circuit>();
auto countA = std::make_shared<Dummy>("countA");
auto countB = std::make_shared<Dummy>("countB");
auto countC = std::make_shared<Dummy>("countC");
circuit->addComponent(countA);
circuit->addComponent(countB);
circuit->addComponent(countC);
circuit->ConnectOutToIn(countA, 0, countB, 0);
circuit->ConnectOutToIn(countB, 0, countC, 0);

circuit->SetBufferCount(3)   // pipeline depth of 3

// manually tick 4 times
circuit->Tick(DSPatch::Component::TickMode::Series);
circuit->Tick(DSPatch::Component::TickMode::Series);
circuit->Tick(DSPatch::Component::TickMode::Series);
circuit->Tick(DSPatch::Component::TickMode::Series);

circuit = nullptr;   // shutdown & destroy the circuit

// examine the number of times the components have been ticked, suprisingly it will be 6
if (countA->numTicks != 4) {
    printf("surprise! number of ticks should be 4, not %d\n", countA->numTicks);
}

The reason this happens can be found in the method Circuit::PauseAutoTick() at L312 of Circuit.cpp.

https://github.com/cross-platform/dspatch/blob/master/src/Circuit.cpp#L312

When the circuit is destroyed, there is no check to see if the circuit was being auto-ticked to begin with. The assumption is that it was, and there seems to be a policy to keep ticking the circuit until the currentThreadNo wraps to zero. Thus, when shutting down the auto-tick thread it appears the aim is to have the circuit be ticked an integer number of times the pipeline depth, which I can understand.

The path to L312 being hit is ~Circuit(), followed by SetBufferCount(0), then PauseAutotick(). So basically destruction of the circuit unconditionally goes through the auto-tick shutdown without checking to see if auto-ticking was active to begin with.

You might ask why a user is not auto-ticking yet having a SetBufferCount > 1. Good question. However, if user is ticking manually, as they might do when debugging, they are rightfully (I would argue) surprised to find that the circuit is being ticked more often than was specifically asked.

Bottom line, at least for me, is that the above-described behaviour is surprising and I would not consider that as correct from a user point of view.

I am available for follow-up discussion.

Again, great work on this library!

Best Regards.

MarcusTomlinson commented 1 year ago

Great catch @axjollivier! I'll have a look

MarcusTomlinson commented 1 year ago

This is fixed now on master :)

axjollivier commented 1 year ago

Super, thanks!