Page 6 of 11

Re: issue-#156 adding Coroutines to BlackBox

PostPosted: Mon Apr 10, 2017 8:17 am
by Josef Templ
handling of very small stack sizes has been improved.
see diffs at https://redmine.blackboxframework.org/projects/blackbox/repository/diff?utf8=%E2%9C%93&rev=d2e29f078214383d4d1d0cf910c832635894ffe6&rev_to=f56119eb4d1cf320603dd78013d8bed82221a0b9.

Ivan, the normal procedure would be to create an alternative branch if you want to show
an alternative solution. Can we agree at least on that?

- Josef

Re: issue-#156 adding Coroutines to BlackBox

PostPosted: Mon Apr 10, 2017 11:18 am
by Ivan Denisov
I made separate branch and added last fix there.
http://blackboxframework.org/unstable/i ... a1.843.zip

I had worked with your branch because I hope on cooperative effect. Why there should be a competition? I think, that such competition will be unnecessary, because my code is very similar to your's. It is just organized in the BlackBox way and it's more easy to port and support.

Coroutines — is the logics of coroutines.
Cross-platform Kernel modification for multiple stacks support.
HostCoroutines — it's realization with Windows Fibers and the new Kernel features.

Re: issue-#156 adding Coroutines to BlackBox

PostPosted: Mon Apr 10, 2017 2:01 pm
by Josef Templ
small improvement in Kernel.AddCoroutine:
stackBase reset to 0.
This enables one to reuse the object for several coroutines.

See diffs at https://redmine.blackboxframework.org/projects/blackbox/repository/diff?utf8=%E2%9C%93&rev=97d6cf15fe06438a50bb095fa275bfaf1d3bf2c0&rev_to=d2e29f078214383d4d1d0cf910c832635894ffe6.

The WinApi calls have been moved to Kernel originally in order to do Ivan a favor.
In the meantime it turned out to be a good move. Why?
Because it is easy this way to guarantee consistency.
If half of the coroutine support is done in the Kernel and the other half is done outside,
it is easily possible to tell the Kernel to switch to coroutine A and via the other half
to switch to coroutine B, creating occasional system crashes in the garbage collector, for example.
By keeping them together this consistency is guaranteed by design.
It also gives the coroutine support routines more flexibility in their implementation
because all aspects are handled at one place.
There is also no runtime overhead by that organization.

The exchangeability need is hard to follow for me.
If there is a much better implementation available it should be used as the default implementation.
Does anybody need multiple implementations at the same time?
I doubt it (and it may create problems if they are mixed at runtime).

The Co_ package has been transformed to use the Kernel's coroutine services now.
There were no surprises, as I expected. A straightforward 1:1 replacement
of WinApi fiber calls by the corresponding Kernel coroutine procedures.
The only surprise was that the Co_ author previously thought that it is not possible.

- Josef

Re: issue-#156 adding Coroutines to BlackBox

PostPosted: Tue Apr 11, 2017 3:11 pm
by Josef Templ
A bug in the Kernel's coroutine garbage collection support has been fixed.
The latest build is http://blackboxframework.org/unstable/issue-%23156/blackbox-1.7.1-a1.847.zip.

- Josef

Re: issue-#156 adding Coroutines to BlackBox

PostPosted: Tue Apr 11, 2017 5:05 pm
by Ivan Denisov
Adopted last Josef's fixes for the branch issue-#156host
http://blackboxframework.org/unstable/i ... a1.848.zip

Josef Templ wrote:The WinApi calls have been moved to Kernel originally in order to do Ivan a favor.
In the meantime it turned out to be a good move. Why?
Because it is easy this way to guarantee consistency.
If half of the coroutine support is done in the Kernel and the other half is done outside,
it is easily possible to tell the Kernel to switch to coroutine A and via the other half
to switch to coroutine B, creating occasional system crashes in the garbage collector, for example.
By keeping them together this consistency is guaranteed by design.
It also gives the coroutine support routines more flexibility in their implementation
because all aspects are handled at one place.

I mentioned the attention on some requirements to any basic feature. It should be split in BlackBox way to cross-platform logics including abstract interface and the realization. You do it some "lazy" way and move the realization to Kernel (mixed stacks manipulations feature with Windows fibers manipulations). It was better then the initial suggestion. However this starts wrong tradition to make Kernel more platform-dependent and "fat". It will be more hard to support it in multiple platforms. Your argument about consistency is good. I understand some benefits. But if all BlackBox will be in one module it will be very consistent AND it will be impossible to deal with it and support it in future. We should always fight with complexity and make decomposition of the complex tasks. Coroutines task is complicated. It includes stack manipulations, fibers manipulations and the logic with some traditional design patterns (Iterator, Task etc.). From my point of view we should split this task:
Coroutines — is the logics of coroutines.
Cross-platform Kernel modification for multiple stacks support.
HostCoroutines — it's realization with Windows Fibers and the new Kernel features.

Josef Templ wrote:There is also no runtime overhead by that organization.

I should mention that there is no runtime overhead by this organization. I had mentioned even 10% faster execution of ObxCoroutines.RunPrimes. This should be checked carefully.

Re: issue-#156 adding Coroutines to BlackBox

PostPosted: Tue Apr 11, 2017 5:18 pm
by Zinn
Ivan Denisov wrote:I mentioned the attention on some requirements to any basic feature. It should be split in BlackBox way to cross-platform logics including abstract interface and the realization...


Ivan, there are more than one design pattern inside BlackBox.
The philosophy of Oberon is "Everything should be as simple as possible, but not simpler" (Einstein).

I am not an expert in coroutines but Josef's 'consistency argument' is convincing to me.
HostCoroutines only makes it more complicated and dangerous to use.

- Helmut

Re: issue-#156 adding Coroutines to BlackBox

PostPosted: Tue Apr 11, 2017 5:39 pm
by Ivan Denisov
Zinn wrote:
Ivan Denisov wrote:I mentioned the attention on some requirements to any basic feature. It should be split in BlackBox way to cross-platform logics including abstract interface and the realization...


Ivan, there are more than one design pattern inside BlackBox.
The philosophy of Oberon is "Everything should be as simple as possible, but not simpler" (Einstein).

I am not an expert in coroutines but Josef's 'consistency argument' is convincing to me.
HostCoroutines only makes it more complicated and dangerous to use.

I understands that everyone can not dive into details. Both versions are worthwhile and it is good that Center members has some choice.

For me this discussion is critical, because after 1.7.1 will be released I have to sync and release version for Linux, OpenBSD and FreeBSD. It will be much easier to concentrate only on the system part. The kernel patch will be trivial, because it can be cross-platform.

To make it easier to distinguish these versions of sources compositions I will past few sources parts here:

Josef's composition:
Kernel
Code: Select all
   PROCEDURE RemoveCoroutine*(this: Coroutine);
      VAR t: TrapCleaner; chk: Handler;
   BEGIN
      IF this.next # NIL THEN
         this.prev.next := this.next; this.next.prev := this.prev;
         this.next := NIL; this.prev := NIL
      END;
      IF (this # currentCoroutine) & (this.fiber # 0) THEN
         WinApi.DeleteFiber(this.fiber); this.fiber := 0;
         WHILE this.trapStack # NIL DO
            t := this.trapStack; this.trapStack := t.next; t.Cleanup
         END;
         chk := this.trapChecker; this.trapChecker := NIL;
         IF (chk # NIL) & (err # 128) THEN chk() END
      END;
   END RemoveCoroutine;


Ivan's composition:
HostCoroutines
Code: Select all
   PROCEDURE (h: Hook) RemoveCoroutine*(this: Coroutines.Impl);
   BEGIN
      WITH this: FiberCoroutine DO
         IF (this # currentCoroutine) & (this.fiber # 0) THEN
            WinApi.DeleteFiber(this.fiber); this.fiber := 0;
            Kernel.RemoveStackInfo(this.stackInfo)
         END
      END
   END RemoveCoroutine;
Kernel
Code: Select all
   PROCEDURE RemoveStackInfo*(s: StackInfo);
   VAR t: TrapCleaner; chk: Handler;
   BEGIN
      IF s.next # NIL THEN
         s.prev.next := s.next; s.next.prev := s.prev; s.next := NIL; s.prev := NIL
      END;
      WHILE s.trapStack # NIL DO
         t := s.trapStack; s.trapStack := t.next; t.Cleanup
      END;
      chk := s.trapChecker; s.trapChecker := NIL;
      IF (chk # NIL) & (err # 128) THEN chk() END
   END RemoveStackInfo;


Josef's composition:
Kernel
Code: Select all
   PROCEDURE TransferCoroutine*(target: Coroutine);
   BEGIN
      ASSERT(target.next # NIL, 20);
      IF target # currentCoroutine THEN
         currentCoroutine.trapStack := trapStack; currentCoroutine.trapChecker := trapChecker;
         S.GETREG(FP, currentCoroutine.stackTop);
         trapStack := target.trapStack; trapChecker := target.trapChecker;
         currentCoroutine := target;
         WinApi.SwitchToFiber(target.fiber)
      END
   END TransferCoroutine;


Ivan's composition:
HostCoroutines
Code: Select all
   PROCEDURE (h: Hook) TransferCoroutine*(target: Coroutines.Impl);
   BEGIN
      WITH target: FiberCoroutine DO
         IF currentCoroutine # target THEN
            Kernel.SwitchStackInfo(currentCoroutine.stackInfo, target.stackInfo);
            currentCoroutine := target;
            WinApi.SwitchToFiber(target.fiber)
         END
      ELSE END
   END TransferCoroutine;
Kernel
Code: Select all
   PROCEDURE SwitchStackInfo*(from, to: StackInfo);
   BEGIN
      ASSERT(to.next # NIL, 20);
      ASSERT(to # currentStack, 21);
      from.trapStack := trapStack; from.trapChecker := trapChecker;
      S.GETREG(FP, from.stackTop);
      trapStack := to.trapStack; trapChecker := to.trapChecker;
      currentStack := to
   END SwitchStackInfo;


The main advantage of my composition that we have no extra WinApi calls in the System modules. Second benefit is that the solution is decomposed so it easier to understand the sources.
The third advantage, that you can improve this HostCoroutines for your aims. For example add some logging of fibers statistics.

Re: issue-#156 adding Coroutines to BlackBox

PostPosted: Wed Apr 12, 2017 9:12 am
by Josef Templ
Ivan Denisov wrote:For me this discussion is critical, because after 1.7.1 will be released I have to sync and release version for Linux, OpenBSD and FreeBSD. It will be much easier to concentrate only on the system part. The kernel patch will be trivial, because it can be cross-platform.


Firstly, porting coroutines is not something that is done every day.
Secondly, for the port to Linux etc. it doesn't make a difference if you edit it here or there.
The real work (99%) is finding the best implementation for Linux etc. and this is the same either way.
The WinApi calls are clearly visible in the source and the compiler marks
every occurrence. It is not possible to overlook it.
This is the same as with other WinApi calls in the Kernel and this is IMHO not a design mistake
that needs to be fixed but a necessary compromise.

Calling anything indirectly with some hook plus additional code can never be faster
than calling directly. If your measurements show something else this must be caused
by something else, e.g. garbage collector invocation or cache effects or other
Windows background activities.

Your RemoveCoroutine version has a different behaviour than mine.

- Josef

Re: issue-#156 adding Coroutines to BlackBox

PostPosted: Wed Apr 12, 2017 9:23 am
by DGDanforth
Your RemoveCoroutine version has a different behaviour than mine.


Picky English language comment: A large proportion of American talkers do
say "different than" but that is not correct. One should say
"different from". Its easy to remember since "different" has "f" in it.

-Doug

Re: issue-#156 adding Coroutines to BlackBox

PostPosted: Wed Apr 12, 2017 9:33 am
by Ivan Denisov
Josef Templ wrote:Your RemoveCoroutine version has a different behaviour...

I just realize that your version is inconsistent... it can remove StackInfo from the list and then does not remove Fiber. Why did you do it this way?