issue-#156 adding Coroutines to BlackBox

Merged to the master branch
Post Reply
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#156 adding Coroutines to BlackBox

Post by Josef Templ »

handling of very small stack sizes has been improved.
see diffs at https://redmine.blackboxframework.org/p ... d82221a0b9.

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
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: issue-#156 adding Coroutines to BlackBox

Post 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.
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#156 adding Coroutines to BlackBox

Post 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/p ... 635894ffe6.

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
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#156 adding Coroutines to BlackBox

Post by Josef Templ »

A bug in the Kernel's coroutine garbage collection support has been fixed.
The latest build is http://blackboxframework.org/unstable/i ... a1.847.zip.

- Josef
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: issue-#156 adding Coroutines to BlackBox

Post 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.
Zinn
Posts: 476
Joined: Tue Mar 25, 2014 5:56 pm
Location: Frankfurt am Main
Contact:

Re: issue-#156 adding Coroutines to BlackBox

Post 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
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: issue-#156 adding Coroutines to BlackBox

Post 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.
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#156 adding Coroutines to BlackBox

Post 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
User avatar
DGDanforth
Posts: 1061
Joined: Tue Sep 17, 2013 1:16 am
Location: Palo Alto, California, USA
Contact:

Re: issue-#156 adding Coroutines to BlackBox

Post 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
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: issue-#156 adding Coroutines to BlackBox

Post 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?
Post Reply