Refactor the convention for coroutines #1

forward-jt · 2021-04-14T07:12:04Z

The macros of coroutine provided in the original version may cause
misusing, such as declaring a set of coroutines by wrapping several
pairs of cr_begin and cr_end macro in a loop, which may cause
unexpected result while someone waits a condition to pass with
cr_wait macro and that condition failed. To avoid such misusing, we
have to abstract the usage of controlling macros that would be invoked
in the coroutine function.
With in this version, a set of macros are introduced, which aim to
provide standard style for someone to declare or access the coroutine
function and its corresponding context.
One should declare a coroutine function with cr_func_def macro, and
its corresponding context with cr_context macro, the argument name
provides to those macros for the same coroutine should be identical.
After declaration, one could run a coroutine by cr_run macro.
Wrapping a set of cr_run macros in a loop makes several coroutines
execute concurrently.
Since the declaration of a coroutine funciton is wrapped by macro
cr_func_def, there is no need to provide the context as argument of
macros that contorl the coroutine, such as cr_begin, cr_end, cr_wait
etc. That argument was hardcoded as __ct in the body of those macros.
To pass arguments to a coroutine, one can provide a pointer to that
argument as parameter of cr_context_init macro, and to extract them
in the coroutine, two macros, cr_arg and cr_arg_member, were
introduced. Macro cr_arg will cast __ct->arg to the pointer of
corresponding type, while cr_arg_member will return a pointer to the
specific member of given structure type that __ct->arg points to.
Finally, to define a static variable with in the coroutine, one can
define them with cr_local macro, which helps to hightlight wheather
a variable is a part of that coroutine or just for temporary usage.
With the changes made in this version, compiler will raise errors for
one of the following situations:

Declaring coroutines as several pairs of cr_begin and cr_end with in a loop.
The declaration of function of a coroutine or its corresponding context was missed.

tinync/tinync.c

jserv · 2021-04-15T03:08:15Z

tinync/tinync.c

-            goto *(o)->label;           \
+
+#define cr_func_name(name) __cr_func_##name
+#define cr_define(name) static void cr_func_name(name)(struct cr * ctx)


Let's respect the unified C-style function declaration as possible.

That is, avoid the indirection such as cr_define.

I intent to keep cr_define since its main purpose is to avoid misusing to coroutine controlling macros, but with some proper modifications, using to those macros could become more C-style from now. Following is my plan to extend cr_define:

modify the content of cr_define as #define cr_define(name, ...) cr_func_name(name)(struct cr * ctx, ## __VA_ARGS_), which makes programmers define a coroutine function as type cr_define(coroutine_name, /* argumnet list */) that allows programmers to specify custom argument list for specific function.

Respond to previous change, content of cr_run should be modified as #define cr_run(name, ...) cr_func_name(name)(&cr_context_name(name), ##__VA_ARGS__).

These modifications makes those indirect macros become more flexible while keeping programmers from misusing controlling macros.

We can use certain prototype macros instead. Let's say, cr_proto, which generates the appropriate function prototype for coroutines.
See https://stackoverflow.com/questions/57779123/whats-the-point-of-a-prototype-macro-that-merely-expands-to-its-arguments

You mean to use macro to generate argument list instead of entire declaration of function?

Assume cr_proto is defined. I expect the use as following:

static void cr_proto(fetch_data, int x) { ... }

Then, the actual declaration would be:

static void __cr_func_fetch_data(int x) { ... }

Thank for your explanation, I think I figure out what you expect for. I will modify those macros to fulfill that.

By the way, coroutine is indeed a common technique. You can check another project which applies the coroutine technique: https://github.com/sysprog21/timeout/blob/master/timeout.c#L576

By the way, coroutine is indeed a common technique. You can check another project which applies the coroutine technique: https://github.com/sysprog21/timeout/blob/master/timeout.c#L576

I notice that a switch case statement is used to control the execution point, rather than labels as values in this project. I'll spend time to figure out pros and cons between these techniques.

Comparisons:

Local continuation based on switch/case and line numbers.

Pros: works with any C99 compiler, most simple implementation.

Cons: does not allow more than one label per line, does not preserve local variables.

Local continuation based on goto label references.

Pros: works with all control structures. slightly faster.

Cons: requires GCC or Clang, does not preserve local variables.

Feel free to contribute to sysprog21/timeout.

jserv · 2021-04-16T14:10:17Z

I changed the subject of this pull request, and please squash the git commits into the one which reflects what you have been been working on.

forward-jt · 2021-04-19T02:17:50Z

I changed the subject of this pull request, and please squash the git commits into the one which reflects what you have been been working on.

I just squashed these commits into one, please have a look.

jserv

Fix the grammar errors and typo:

Since for the original version, programmers may misusing these macros,
to avoid such situation, we have modified macros that are used for
controlling coroutines and macros that programmers use for declaring
coroutine funcitons and their corresponding contexts.

Should be "misuse" and "functions"

jserv · 2021-04-19T04:45:21Z

Let's simplify the description:

we have introduced a macro called cr_proto, which refers to the generator for function prototype for coroutine funcitons. Any coroutine function must be declared via this macro.

You can say, "cr_proto was introduced as the generator of coroutine function declarations, and all coroutines should be specified with the macro."

Since for the original version, programmers may misuse these macros, to avoid such situation, we have modified macros that are used for controlling coroutines and macros that programmers use for declaring coroutine functions and their corresponding contexts. First of all, cr_proto was introduced as the generator of coroutine function declarations, and all coroutines should be specified with this macro. By introducing cr_proto, we have wrapped the argument that refers to the coroutine context into a fixed name variable called ctx, which will be refered by controlling macros such as cr_begin, cr_end, cr_wait and so on. Invoking these macros is valid only in a coroutine function that was generated by cr_proto, otherwise, compiler will report errors. Next, we have modified structure cr that its field local is removed, and macro cr_context is introduced for programmers to declare the coroutine contexts. The name assigned to cr_context should be identical with the one used to generate the coroutine function. After generated a coroutine function and its corresponding context, programmers could launch that coroutine via macro cr_run. Finally, to declare static variables in coroutine, macro cr_local is introduced, which helps to mark wheather a variable is a part of coroutine or just for temporary usage. Abstract using of coroutine macros With in this version, a set of macros are introduced, which aim to provide standard style for someone declare or access the coroutine function and its corresponding context. One should declare a coroutine function with cr_func_def macro, and its corresponding context with cr_context macro, the argument name provides to those macros for the same coroutine should be identical. After declaration, one could run a coroutine by cr_run macro. Wrapping a set of cr_run macros in a loop makes several coroutines execute concurrently. Since the declaration of a coroutine funciton is wrapped by macro cr_func_def, there is no need to provide the context as argument of macros that contorl the coroutine, such as cr_begin, cr_end, cr_wait etc. That argument was hardcoded as __ct in the body of those macros. To pass arguments to a coroutine, one can provide a pointer to that argument as parameter of cr_context_init macro, and to extact them in the coroutine, two macros, cr_arg and cr_arg_member, were introduced. Macro cr_arg will cast __ct->arg to the pointer of corresponding type, while cr_arg_member will return a pointer to the specific member of given structure type that __ct->arg points to. Finally, to define a static variable with in the coroutine, one can define them with cr_local macro, which helps to hightlight wheather a variable is a part of that coroutine or just for temporary usage. Rename macros and variables To make the name of macros and variables indicate purpose of themself more clearly, some of them were renamed. Following lists the modified macros and variables: * Macro cr_func_def is renamed as cr_define, which will avoid confusing and make programmers focus on dealing body of coroutine. * Hardcoded __ct is renamed as ctx which stands for context.\ * The name of structure sock_w_loc_t is changed to conn_data_t, since it aims to hold data used for connection. Make coroutine function be declared in C-Style Macro cr_proto is introduced to replace cr_define, which makes programmers to declare a coroutine function in C-style, that is, more flexible than cr_define. Macro cr_run is modified to make respond to cr_proto. The new version of cr_run accepts variable length argument list. From now, programmers could specify arguments that are going to be passed to coroutines, by listing arguments after name while starting coroutine with macro cr_run. Since the way of passing arguments was changed, the field arg in structure cr is removed and macros that aimed to fetch value from arg are also removed.

forward-jt · 2021-04-19T10:59:29Z

Thank you for pointing out my grammar errors and typos, I just fixed those errors and simplified the description with your suggestion, please have a look.

jserv · 2021-04-19T11:57:22Z

Thank you for pointing out my grammar errors and typos, I just fixed those errors and simplified the description with your suggestion, please have a look.

It looks great to me. Please write README.md as well.

Add the missing suffix .o after one of tinync of target clean.

forward-jt · 2021-04-20T01:13:38Z

README.md is introduced and a missing suffix .o in Makefile is added.

jserv · 2021-04-20T01:50:21Z

tinync/README.md

+    /* b and r are variables used in coroutine whose
+     * value will be preserved across pauses.
+     */
+    cr_local uint9_t b;


Sorry for the mistyped, I will fix that.

jserv · 2021-04-20T01:53:02Z

tinync/README.md

+### Declaring a Coroutine Function
+At the very beginning, a coroutine function should be declared first, coroutine function is part of program that will be executed simultaneously with other coroutines. All coroutines should be specified with the macro `cr_proto`, following shows the example:
+```cpp
+static void cr_proto(coroutine_name, int x, float y)


Replace "int x, float y" with generic argument list, such as parameter_declarations.
see http://www.cplusplus.com/reference/cstdarg/

jserv · 2021-04-20T03:09:43Z

tinync/README.md

@@ -0,0 +1,78 @@
+# Tinync
+Tinync is a simplified version of [nc](https://en.wikipedia.org/wiki/Netcat), which aims to demonstrate the usage of coroutine macros and their effects.


Add hyperlink to coroutine as well.

jserv · 2021-04-20T03:10:13Z

tinync/README.md

+# Tinync
+Tinync is a simplified version of [nc](https://en.wikipedia.org/wiki/Netcat), which aims to demonstrate the usage of coroutine macros and their effects.
+
+## Usage


"Usage" was misleading. You can say, "internals."

jserv · 2021-04-20T03:10:52Z

tinync/README.md

+    cr_local uint8_t b;
+    cr_local int r;
+
+    cr_begin(); // Initiates the context of this coroutine.


Change to C-style comment. That is, /* ... */

jserv · 2021-04-20T03:11:45Z

tinync/README.md

+```
+
+### Coroutine Context
+There is another important part of coroutine, that is, context. Context of a coroutine will preserve its execution point which could be resumed later. To define a context for a coroutine, use `cr_context` macro and initiates it with macro `cr_context_init`. It is important to **assign an identical name to context and its corresponding function**. With example presented at the beginning, its corresponding context should be specified as follows:


When you talk about "another important part," there is no evident "important part" before.

jserv · 2021-04-20T04:54:43Z

tinync/README.md

 }
 ```

 ### Coroutine Context
-There is another important part of coroutine, that is, context. Context of a coroutine will preserve its execution point which could be resumed later. To define a context for a coroutine, use `cr_context` macro and initiates it with macro `cr_context_init`. It is important to **assign an identical name to context and its corresponding function**. With example presented at the beginning, its corresponding context should be specified as follows:
+Context is an important part for coroutine, which will preserve the execution point of a coroutine that could be resumed later. To define a context for a coroutine, use `cr_context` macro and initiates it with macro `cr_context_init`. It is important to **assign an identical name to context and its corresponding function**. With example presented at the beginning, its corresponding context should be specified as follows:


The statement "which will preserve" is Inconsistent tense. Change it to something like "which preserves."

jserv · 2021-04-20T04:55:12Z

tinync/README.md

@@ -1,7 +1,7 @@
 # Tinync
-Tinync is a simplified version of [nc](https://en.wikipedia.org/wiki/Netcat), which aims to demonstrate the usage of coroutine macros and their effects.
+Tinync is a simplified version of [nc](https://en.wikipedia.org/wiki/Netcat), which aims to demonstrate the usage of [coroutine](https://en.wikipedia.org/wiki/Coroutine) macros and their effects.


Replace "simplified version" with "simplified implementation."

jserv · 2021-04-20T04:56:44Z

tinync/README.md

    for (;;) {
-        cr_sys(r = read(STDIN_FILENO, &b, 1)); // Wait for read system call to be success.
+        /* Wait for read system call to be success. */


"be success" is grammatically incorrect.

jserv · 2021-04-20T04:57:48Z

After you improves the documentation, do squash git commits. Then, I am about to merge this pull request.

README.md is introduced to help programmers to figure out the usage of coroutine macros. Add README.md README.md is introduced to help programmers to figure out the usage of coroutine macros. Fix typo of README.md Modify README.md Following modifications are performed: * Add hyperlink to coroutine. * Use word "Internal" instead of "Usage" * Change to C-style comment. * Modify the description in Coroutine Context section. Improve README.md

forward-jt · 2021-04-20T06:04:41Z

After you improves the documentation, do squash git commits. Then, I am about to merge this pull request.

Thank you for your acceptance. I just improved README.md and squashed commits on it, please have a look.

jserv · 2021-04-20T06:17:46Z

Thank @RZHuangJeff for contributing!

Currently, the reader_func is counting the number of grace periods based on the value of dut, which is updated by mthpc_rcu_replace_pointer. However, the value of dut actually represents the time we update the value, not the number of grace periods. Also, the original method might result in incorrect counting if someone tried to update the gp_idx while others who saw the same dut value with prev_count still depend on the old gp_idx to increase the counter. To fix the problem, instead of relying on the dut value to increase the gp_idx, we manually increase gp_idx on write side. Then, we can easily determine the gp on read side. For dut value, we simply check the old count value is not greater than the newest one. Additionally, since synchronize_rcu is quite slow, readers generally will pass through the critical section during the first grace period. To generate more realistic output, we add a delay on read side before entering the critical section. Before: 100 reader(s), 5 update run(s), 6 grace period(s) [grace period #0] 100 reader(s) [grace period sysprog21#1] 0 reader(s) [grace period sysprog21#2] 0 reader(s) [grace period sysprog21#3] 0 reader(s) [grace period sysprog21#4] 0 reader(s) [grace period sysprog21#5] 0 reader(s) After, we added a delay: 100 reader(s), 5 update run(s), 6 grace period(s) [grace period #0] 76 reader(s) [grace period sysprog21#1] 0 reader(s) [grace period sysprog21#2] 1 reader(s) [grace period sysprog21#3] 0 reader(s) [grace period sysprog21#4] 3 reader(s) [grace period sysprog21#5] 20 reader(s)

Currently, the reader_func is counting the number of grace periods based on the value of dut, which is updated by rcu_assign_pointer. However, the value of dut actually represents the time we update the value, not the number of grace periods. Also, the original method might result in incorrect counting if someone tried to update the gp_idx while others who saw the same dut value with prev_count still depend on the old gp_idx to increase the counter. To fix the problem, instead of relying on the dut value to increase the gp_idx, we manually increase gp_idx on write side. Then, we can easily determine the gp on read side. For dut value, we simply check the old count value is not greater than the newest one. Additionally, since synchronize_rcu is quite slow, readers generally will pass through the critical section during the first grace period. To generate more realistic output, we add a delay on read side before entering the critical section. Before: 100 reader(s), 5 update run(s), 6 grace period(s) [grace period #0] 100 reader(s) [grace period sysprog21#1] 0 reader(s) [grace period sysprog21#2] 0 reader(s) [grace period sysprog21#3] 0 reader(s) [grace period sysprog21#4] 0 reader(s) [grace period sysprog21#5] 0 reader(s) After, we added a delay: 100 reader(s), 5 update run(s), 6 grace period(s) [grace period #0] 76 reader(s) [grace period sysprog21#1] 0 reader(s) [grace period sysprog21#2] 1 reader(s) [grace period sysprog21#3] 0 reader(s) [grace period sysprog21#4] 3 reader(s) [grace period sysprog21#5] 20 reader(s)

jserv reviewed Apr 14, 2021

View reviewed changes

tinync/tinync.c Outdated Show resolved Hide resolved

jserv reviewed Apr 14, 2021

View reviewed changes

tinync/tinync.c Outdated Show resolved Hide resolved

jserv reviewed Apr 15, 2021

View reviewed changes

jserv changed the title ~~Abstract using of coroutine macros~~ Refactor the convention for coroutines Apr 16, 2021

forward-jt force-pushed the tinync-imprv branch from 6d29ac0 to e4897a9 Compare April 19, 2021 02:13

jserv requested changes Apr 19, 2021

View reviewed changes

forward-jt force-pushed the tinync-imprv branch from e4897a9 to 6f38fb5 Compare April 19, 2021 10:56

Fix error in Makefile

bcb985f

Add the missing suffix .o after one of tinync of target clean.

jserv reviewed Apr 20, 2021

View reviewed changes

forward-jt force-pushed the tinync-imprv branch from e1dbe77 to 30f7f8b Compare April 20, 2021 05:57

jserv merged commit 9a042a6 into sysprog21:master Apr 20, 2021

		@@ -0,0 +1,78 @@
		# Tinync
		Tinync is a simplified version of [nc](https://en.wikipedia.org/wiki/Netcat), which aims to demonstrate the usage of coroutine macros and their effects.

Refactor the convention for coroutines #1

Refactor the convention for coroutines #1

Uh oh!

Conversation

forward-jt commented Apr 14, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

forward-jt Apr 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jserv commented Apr 16, 2021

Uh oh!

forward-jt commented Apr 19, 2021

Uh oh!

jserv left a comment

Choose a reason for hiding this comment

Uh oh!

jserv commented Apr 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

forward-jt commented Apr 19, 2021

Uh oh!

jserv commented Apr 19, 2021

Uh oh!

forward-jt commented Apr 20, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jserv commented Apr 20, 2021

Uh oh!

forward-jt commented Apr 20, 2021

Uh oh!

jserv commented Apr 20, 2021

Uh oh!

Uh oh!

forward-jt Apr 15, 2021 •

edited

Loading

jserv commented Apr 19, 2021 •

edited

Loading