support icon indicating copy to clipboard operation
support copied to clipboard

[Bug] Improve async loop time

Open laurensvalk opened this issue 1 year ago • 8 comments

The loop time doesn't seem to work as intended which is a separate issue.

Originally posted by @laurensvalk in https://github.com/pybricks/support/issues/1429#issuecomment-1952034238

laurensvalk avatar Feb 19 '24 09:02 laurensvalk

The following works as "intended", but this is somewhat questionable. The async loop time is set to 10 ms.

from pybricks.tools import run_task, wait, StopWatch

watch = StopWatch()

async def main1():
    while True:
        await wait(1)
        print(watch.time())

run_task(main1())
10
20
30
40
50
60
70
80
90

laurensvalk avatar Feb 19 '24 09:02 laurensvalk

On second thought, the issue I thought I was originally seeing wasn't due to async, so this is not as critical as I thought.

It's probably still worth considering the 10 ms loop though, as it would slow down plain-math loops in block code, since they always have one yield or (await 0) auto-inserted.

laurensvalk avatar Feb 19 '24 09:02 laurensvalk

Another possibility is to leave it at 10 ms, and provide an option to avoid auto-inserting loop trap yields for more advanced users.

laurensvalk avatar Feb 19 '24 09:02 laurensvalk

Also, it would be nice to avoid inserting it if there is already a known, nonzero constant wait in the loop at the same nesting level. We should be able to detect this quite easily since we know the inner stack of block when we generate the code for the loop. EDIT: We might only be able to do this if that wait is guaranteed to run (e.g. sits at the top).

Right now, if you have a 1 ms loop and you then switch to multitask (which adds another 1ms wait), the loop actually takes 20 ms, since the async loop time is 10ms right now.

laurensvalk avatar May 31 '24 10:05 laurensvalk

Could we expose the loop time in the "set up" hat block? I.e. If there is only one "program" hat block, nothing changes. If there is more than one "program" hat block then "set up" grows an extra parameter for the loop time. This would make it somewhat obvious that there is an implicit loop time shared by all parallel tasks. And that way, people could tweak the default 10ms loop time if needed.

dlech avatar May 31 '24 13:05 dlech

That's a good idea.

I was thinking about having some kind of extra (and thereby optional) block to configure that sort of thing. Besides the async loop time, we could allow users to configure if (and/or how much) waits are injected in loops and functions.

Power users could opt to skip inserting waits. Right now, iterating through a list to do some operation also takes 10ms each iteration for example, which is probably undesired in (finite) data processing cases.

laurensvalk avatar May 31 '24 17:05 laurensvalk

In https://github.com/pybricks/support/issues/1780 it was suggested to drop the loop_time arg. This is done in https://github.com/pybricks/pybricks-micropython/pull/263. Combined with the fix for wait(0), this issue is fixed.

What remains then is to change the block code generator to insert wait(0) instead of wait(1).

laurensvalk avatar Aug 27 '24 08:08 laurensvalk

What remains then is to change the block code generator to insert wait(0) instead of wait(1).

This isn't done yet, so re-opening.

laurensvalk avatar Aug 28 '24 12:08 laurensvalk