search

trustmaster / goflow

Flow-based and dataflow programming library for Go (golang)
MIT License
1.6k stars 125 forks source link

Remove InputGuard, refactor test components without it #64

Closed akokhanovskyi closed 3 years ago

akokhanovskyi commented 3 years ago

Rationale: InputGuard is unsafe to use; components for test provide a bad example. Explanation: The key problem is that after a channel is closed, it never blocks reads. The existing implementation leads to a dead select loop, which eventually exits when select randomly chooses another execution branch. Updated component implementations provide a more accurate example of handling closed channels in go. Additionally, keeping a map of inputs is expensive and totally unnecessary, as it can be seen from the updated impl. Finally, if we were to preserve InputGuard-s for some reason, the implementation would have to be changed for safety. Consider what would happen if you accidentally Complete()-d a non-existing port. What would happen if you accidentally Complete()-d the same port twice?

trustmaster commented 3 years ago

@akokhanovskyi I agree InputGuard is error-prone and probably it isn't straight-forward either.

Now that we are at it, I'd like to ask for your ideas about solving the original problem that InputGuard was targeting. The problem is not handling closed channels, the problem is simplifying handling of multiple channels in the same process. InputGuard was my first experiment in this area.

The most typical situation is the following. We have some component C which has multiple inports, e.g. In1, In2, In3. Behaviour of the component is: read inputs in1, in2, in3 on all of its inputs, and then apply some function f(in1, in2, in3) to get the result. Typically, result is then sent to its output. Once all of its input channels are closed, the process terminates.

Components like this are so common, that doing manual select logic every time is very repetitive. Ideally, there should be some easy to use API for this. And ideally, it shouldn't use reflection, because using reflection on every activation of a process is expensive (this was the main critique for GoFlow v0).

akokhanovskyi commented 3 years ago

Right, staying low on reflection makes perfect sense. So far, the best option that comes to my mind is to provide some code generation tool that would reduce the amount of boilerplate to be written. I am not ready to comment on the functionality or usefulness of such a tool, though.

On the other hand, could a few small examples illustrating typical use cases fit the bill as the MVP? I could use a few of those myself when starting with goflow :)

trustmaster commented 3 years ago

I tried to think of a solution on the weekend but could not come up with anything that would work without generics or reflection. I guess we can do with heavy boilerplate for now.

As per examples, all I have currently is the tests and #53 which I'm using as a playground. This is all I can offer at this point, because my time to work on this project is very limited.