search

POETSII / Orchestrator

The Orchestrator is the configuration and run-time management system for POETS platforms.
1 stars 1 forks source link

No support for requestIdle #234

Closed m8pple closed 3 years ago

m8pple commented 3 years ago

The v4 spec defined a variable requestIdle which is used to indicate whether the device would like the OnDeviceIdle handler to run:

https://github.com/POETSII/poets_improvement_proposals/blob/2a355282f3aa3912bdacf4ec50bf25199bdf310d/proposed/PIP-0020/virtual-graph-schema-v4.rnc#L522-L526

The orchestrator documentation doesn't talk about OnDeviceIdle, though it seems to parse it fine, and it is there in the grammar ocfg file. Instead the documentation talks about OnIdle, which isn't supported by the orchestrator:

https://github.com/POETSII/orchestrator-documentation/blob/1.0.0-alpha/source/applications.md#defining-device-and-pin-behaviours

OnIdle: Invoked when there is nothing to receive, and nothing to send. The Softswitch will execute the OnIdle behaviour for each device it manages in sequence, until either a packet can be received, or OnIdle has been executed for all hosted devices. If any of the OnIdle application-writer-supplied fragments returned a non-zero unsigned value, that indicates the device may "want" to send a packet.

That is quite different behaviour than has been discussed before, and has some potential performance implications, as potentially every time a packet is received the softswitch might run OnIdle. Even if OnIdle immediately returns, that execution still takes up some time in the pipeline, and will use CPU time that other threads could be using to receive packets. So it would be better if the device indicates whether it wants the opportunity to do idle compute, rather than giving repeatedly give it the opportunity and then it has to keep saying no.

That was the idea behind requestIdle, in that the device explicitly indicates that it wants to compute. It has the nice side-effect that if you do cross module optimisation, then the compiler will realise that requestIdle is never set, and can strip out the code-path. Though I guess the orchestrator could be doing similar optimisations based on whether the OnDeviceIdle element is empty or not.

Further down the documentation we get:

Graphs/GraphType/DeviceTypes/DeviceType/OnDeviceIdle (:OnDeviceIdle:)

Contains code that is executed by the device when the softswitch is in the "idle" state. Under the default softswitch, this block is executed when no devices have any messages to receive or send. If the code in this block returns a non-zero unsigned value, the code in the ReadyToSend section is executed, which may result in the sending of messages.

This element must occur at most once in each :DeviceType: section. No attributes are valid.

So I'm guessing the OnIdle thing is just old, and it is OnDeviceIdle now. This still talks about "return" as the method used to indicate whether it wants to compute or not, but generally we have always avoided having handlers executing return, and used pointers with well-known names for these purposes. Using return makes some other types of code-gen more complicated, for example the HLS back-end a student did a few years ago would be much more complex if it had to support "return" in a handler.

m8pple commented 3 years ago

This was done on development_dt10_branch (fd0196a), as otherwise I cant get through parsing through to compilation. The branch has the current 1.0.0-alpha merged in.

Test case for this is:

dt10@joxer:~/POETS/Orchestrator$ ./orchestrate.sh 
[WARN] Launcher: Not running on a POETS box, and found no motherships running on alternative machines, so we're not spawning any mothership processes.
POETS> 12:20:43.04:  20(I) The microlog for the command 'load /engine = "../Config/POETSHardwareOneBox.ocfg"' will be written to '../Output/Microlog/Microlog_2021_06_13T12_20_43p0.plog'.
POETS> 12:20:43.04: 140(I) Topology loaded from file ||../Config/POETSHardwareOneBox.ocfg||.
POETS>load /app = "/home/dt10/POETS/Orchestrator/Tests/ReferenceXML/v4/PEP20/apps/example_device_idle.xml"
Msg: m1 = '// Line 8
// CFrag /home/dt10/POETS/Orchestrator/Tests/ReferenceXML/v4/PEP20/apps/example_device_idle.xml.Graphs.GraphType.MessageTypes.MessageType.Message empty
'
POETS> 12:20:46.87:  23(I) load /app = "/home/dt10/POETS/Orchestrator/Tests/ReferenceXML/v4/PEP20/apps/example_device_idle.xml"
POETS> 12:20:46.87:  20(I) The microlog for the command 'load /app = "/home/dt10/POETS/Orchestrator/Tests/ReferenceXML/v4/PEP20/apps/example_device_idle.xml"' will be written to '../Output/Microlog/Microlog_2021_06_13T12_20_46p0.plog'.
POETS> 12:20:46.87: 235(I) Application file /home/dt10/POETS/Orchestrator/Tests/ReferenceXML/v4/PEP20/apps/example_device_idle.xml loading...
POETS> 12:20:46.87:  65(I) Application file /home/dt10/POETS/Orchestrator/Tests/ReferenceXML/v4/PEP20/apps/example_device_idle.xml loaded in 2874 ms.
POETS>tlink /app = *
POETS> 12:20:50.44:  23(I) tlink /app = *
POETS> 12:20:50.44:  20(I) The microlog for the command 'tlink /app = *' will be written to '../Output/Microlog/Microlog_2021_06_13T12_20_50p0.plog'.
POETS>place /tfill = *
POETS> 12:20:53.56:  23(I) place /tfill = *
POETS> 12:20:53.56:  20(I) The microlog for the command 'place /tfill = *' will be written to '../Output/Microlog/Microlog_2021_06_13T12_20_53p0.plog'.
POETS> 12:20:53.56: 309(I) Attempting to place graph instance 'gi' using the 'tfil' method...
POETS> 12:20:53.56: 302(I) Graph instance 'gi' placed successfully.
POETS>compose /app = *
POETS> 12:21:04.80:  23(I) compose /app = *
POETS> 12:21:04.80:  20(I) The microlog for the command 'compose /app = *' will be written to '../Output/Microlog/Microlog_2021_06_13T12_20_59p0.plog'.
POETS> 12:21:04.80: 803(I) Composing graph instance 'gi'...
POETS> 12:21:04.80: 806(W) Graph instance 'gi' compose failed. Check the microlog for details.
POETS>

Microlog is:

========================================================================================================================
13/06/2021 12:20:59.79 file ../Output/Microlog/Microlog_2021_06_13T12_20_59p0.plog
command [compose /app = *]
from console
========================================================================================================================

Composing gi...
    Generating Supervisor...    Done!
    Generating code for 1 cores
    Core 0: Files... Source... 
RTS Buffer for thread 0 expanded from 5 to 10

    Make called with make -j$(nproc --ignore=4) all SOFTSWITCH_TRIVIAL_LOG_HANDLER=1 SOFTSWITCH_LOGLEVEL=2 > make_errs.txt 2>&1
Compilation failed! make_errs.txt dump ++++++++++++++++++++++++++++++++++++++++

riscv32-unknown-elf-g++ -mabi=ilp32 -march=rv32imf -static -mcmodel=medany -fvisibility=hidden -nostartfiles -pipe -fsingle-precision-constant -fno-builtin-printf -ffp-contract=off -std=c++14 -Wall -O2  -DTRIVIAL_LOG_HANDLER -DP_LOG_LEVEL=2 -I /home/dt10/POETS/Orchestrator/Tinsel/include -I /home/dt10/POETS/Orchestrator/Source/Softswitch/inc -I /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/inc -I/home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated -I /home/dt10/POETS/Orchestrator/Generics -I /home/dt10/POETS/Orchestrator/Source/Common -Wall -c -o softswitch.o /home/dt10/POETS/Orchestrator/Source/Softswitch/src/softswitch.cpp
riscv32-unknown-elf-g++ -mabi=ilp32 -march=rv32imf -static -mcmodel=medany -fvisibility=hidden -nostartfiles -pipe -fsingle-precision-constant -fno-builtin-printf -ffp-contract=off -std=c++14 -Wall -O2  -DTRIVIAL_LOG_HANDLER -DP_LOG_LEVEL=2 -I /home/dt10/POETS/Orchestrator/Tinsel/include -I /home/dt10/POETS/Orchestrator/Source/Softswitch/inc -I /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/inc -I/home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated -I /home/dt10/POETS/Orchestrator/Generics -I /home/dt10/POETS/Orchestrator/Source/Common -Wall -c -o vars_0_0.o /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/src/vars_0_0.cpp
riscv32-unknown-elf-g++ -mabi=ilp32 -march=rv32imf -static -mcmodel=medany -fvisibility=hidden -nostartfiles -pipe -fsingle-precision-constant -fno-builtin-printf -ffp-contract=off -std=c++14 -Wall -O2  -DTRIVIAL_LOG_HANDLER -DP_LOG_LEVEL=2 -I /home/dt10/POETS/Orchestrator/Tinsel/include -I /home/dt10/POETS/Orchestrator/Source/Softswitch/inc -I /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/inc -I/home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated -I /home/dt10/POETS/Orchestrator/Generics -I /home/dt10/POETS/Orchestrator/Source/Common -Wall -c -o io.o /home/dt10/POETS/Orchestrator/Tinsel/lib/io.c
riscv32-unknown-elf-gcc -mabi=ilp32 -march=rv32imf -static -mcmodel=medany -fvisibility=hidden -nostartfiles -pipe -fsingle-precision-constant -fno-builtin-printf -ffp-contract=off -std=c++14 -Wall -O2  -DTRIVIAL_LOG_HANDLER -DP_LOG_LEVEL=2 -I /home/dt10/POETS/Orchestrator/Tinsel/include -I /home/dt10/POETS/Orchestrator/Source/Softswitch/inc -I /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/inc -I/home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated -I /home/dt10/POETS/Orchestrator/Generics -I /home/dt10/POETS/Orchestrator/Source/Common -Wall -c -o entry.o /home/dt10/POETS/Orchestrator/Source/Softswitch/inc/entry.S
riscv32-unknown-elf-g++ -mabi=ilp32 -march=rv32imf -static -mcmodel=medany -fvisibility=hidden -nostartfiles -pipe -fsingle-precision-constant -fno-builtin-printf -ffp-contract=off -std=c++14 -Wall -O2  -DTRIVIAL_LOG_HANDLER -DP_LOG_LEVEL=2 -I /home/dt10/POETS/Orchestrator/Tinsel/include -I /home/dt10/POETS/Orchestrator/Source/Softswitch/inc -I /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/inc -I/home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated -I /home/dt10/POETS/Orchestrator/Generics -I /home/dt10/POETS/Orchestrator/Source/Common -Wall -c -o softswitch_main.o /home/dt10/POETS/Orchestrator/Source/Softswitch/src/softswitch_main.cpp
riscv32-unknown-elf-g++ -mabi=ilp32 -march=rv32imf -static -mcmodel=medany -fvisibility=hidden -nostartfiles -pipe -fsingle-precision-constant -fno-builtin-printf -ffp-contract=off -std=c++14 -Wall -O2  -DTRIVIAL_LOG_HANDLER -DP_LOG_LEVEL=2 -I /home/dt10/POETS/Orchestrator/Tinsel/include -I /home/dt10/POETS/Orchestrator/Source/Softswitch/inc -I /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/inc -I/home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated -I /home/dt10/POETS/Orchestrator/Generics -I /home/dt10/POETS/Orchestrator/Source/Common -Wall -c -o softswitch_common.o /home/dt10/POETS/Orchestrator/Source/Softswitch/src/softswitch_common.cpp
riscv32-unknown-elf-g++ -mabi=ilp32 -march=rv32imf -static -mcmodel=medany -fvisibility=hidden -nostartfiles -pipe -fsingle-precision-constant -fno-builtin-printf -ffp-contract=off -std=c++14 -Wall -O2  -DTRIVIAL_LOG_HANDLER -DP_LOG_LEVEL=2 -I /home/dt10/POETS/Orchestrator/Tinsel/include -I /home/dt10/POETS/Orchestrator/Source/Softswitch/inc -I /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/inc -I/home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated -I /home/dt10/POETS/Orchestrator/Generics -I /home/dt10/POETS/Orchestrator/Source/Common -Wall -c -o vars_0.o /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/src/vars_0.cpp
riscv32-unknown-elf-g++ -mabi=ilp32 -march=rv32imf -static -mcmodel=medany -fvisibility=hidden -nostartfiles -pipe -fsingle-precision-constant -fno-builtin-printf -ffp-contract=off -std=c++14 -Wall -O2  -DTRIVIAL_LOG_HANDLER -DP_LOG_LEVEL=2 -I /home/dt10/POETS/Orchestrator/Tinsel/include -I /home/dt10/POETS/Orchestrator/Source/Softswitch/inc -I /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/inc -I/home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated -I /home/dt10/POETS/Orchestrator/Generics -I /home/dt10/POETS/Orchestrator/Source/Common -Wall -c -o handlers_0.o /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/src/handlers_0.cpp
/home/dt10/POETS/Orchestrator/Source/Softswitch/src/genld.sh 0 > link_0.ld
cp /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/supervisor.bin ./
ld -r -b binary -o SupervisorData.o supervisor.bin
rm supervisor.bin
/mnt/e/dt10_all/POETS/orchestrator_dependencies_7/mpich/bin/mpicxx -I/home/dt10/POETS/Orchestrator/Source/Softswitch/inc -I/home/dt10/POETS/Orchestrator/Tinsel/include -I/mnt/e/dt10_all/POETS/orchestrator_dependencies_7/mpich/include -I/home/dt10/POETS/Orchestrator/Source/Common -I/home/dt10/POETS/Orchestrator/Generics  -std=c++14 -fPIC -pipe -Wall -shared -L/mnt/e/dt10_all/POETS/orchestrator_dependencies_7/mpich/lib -O3 -Wl,-soname,libSupervisor.so -o ../bin/libSupervisor.so /home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/Supervisor.cpp SupervisorData.o
/home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/src/handlers_0.cpp: In function 'uint32_t devtyp_dt1_RTS_handler(const void*, void*, uint32_t*)':
/home/dt10/POETS/Orchestrator/Output/Composer/gi__gi/Generated/src/handlers_0.cpp:40:30: error: 'requestIdle' was not declared in this scope
                             *requestIdle = true;
                              ^~~~~~~~~~~
Makefile:192: recipe for target 'handlers_0.o' failed
make: *** [handlers_0.o] Error 1
make: *** Waiting for unfinished jobs....
rm softswitch_main.o softswitch_common.o vars_0.o link_0.ld

Compilation failed! make_errs.txt dump ----------------------------------------
mvousden commented 3 years ago

Regarding the confusing bit of the documentation, that first paragraph refers to the Softswitch's OnIdle behaviour, which is different from the behaviour of the devices' OnDeviceIdle behaviour. I've opened a PR at https://github.com/POETSII/orchestrator-documentation/pull/16 to address this.

mvousden commented 3 years ago

I like the idea of requestIdle, but I question how valuable that is in practice - OnDeviceIdle will only be called once "unneccesarily" for a well-written application, as that first call will return non-zero, which will prevent successive calls to OnDeviceIdle until another packet is received.

If requestIdle is desirable, isn't that functionality best wrapped into a function in Graphs/GraphType/SharedCode, and called from there when requested?

heliosfa commented 3 years ago

Mark has correctly pointed out that there is a play between the Softswitch's OnIdle behaviour and the OnDeviceIdle handler.

Currently, when the Softswitch has nothing to receive, nothing to send [1] and no instrumentation to send, it starts to loop through the OnDeviceIdle handlers of each device it hosts.

At each loop iteration, it first checks whether there is something to receive. If there is, it marks that "something happened" and breaks out of the loop (saving the position), the received packet is then dealt with.

If the Softswitch iterates over all of the OnDeviceIdle handlers uninterrupted and none of them "returned" 1, then nothing "interesting" happened and the Softswitch blocks on a TinselWaitUntil(CanRecv).

If any of the handlers "returned" 1, then something "interesting" happened and the ReadyToSend handler is executed for that device.

This flow chart shows the loop: image

The full Softswitch/Supervisor/Composer docs can be found here:

Even if OnIdle immediately returns, that execution still takes up some time in the pipeline, and will use CPU time that other threads could be using to receive packets. So it would be better if the device indicates whether it wants the opportunity to do idle compute, rather than giving repeatedly give it the opportunity and then it has to keep saying no.

No matter how we cut this, the Softswitch is going to have an overhead checking for idle. e.g. two approaches I can think of off the top of my head:

Though I guess the orchestrator could be doing similar optimisations based on whether the OnDeviceIdle element is empty or not.

We are not currently doing this. A "stub" idle handler is created irrespective of whether there is content or not.

This still talks about "return" as the method used to indicate whether it wants to compute or not, but generally we have always avoided having handlers executing return, and used pointers with well-known names for these purposes. Using return makes some other types of code-gen more complicated, for example the HLS back-end a student did a few years ago would be much more complex if it had to support "return" in a handler.

Yep, it still does use "return" to indicate whether the device's ReadyToSend handler should be called following the OnDeviceIdle handler and it is used to determine whether the Softswitch can go to sleep completely (e.g. block on tinselWaitUntil(CanRecv)) or whether it needs to go round the loop again and potentially process sends triggered from OnIdle.

We also (still) use a return in the OnInit to selectively trigger ReadyToSend.

There is nothing stopping us defining a pointer as is used for ReadyToSend for both of these use cases - not that much effort, we just need to define the pointer names. Alternatively we could just take the stance of "call ReadyToSend after everything", but that is potentially a lot of extra conditionals and memory access.

  1. In our Softswitch, if we want to send but can't, we block on a tinselWaitUntil(CanRecv|CanSend) and do not enter OnIdle processing. In other words, we completely yield until whatever is stopping us sending has cleared or we have to receive something.
heliosfa commented 3 years ago

Fixed in #274