MightyPirates / OpenComputers

Home of the OpenComputers mod for Minecraft.
https://oc.cil.li
Other
1.58k stars 430 forks source link

os.sleep often sleeps longer than asked for #1667

Closed SoniEx2 closed 7 years ago

SoniEx2 commented 8 years ago

I'm trying to write a NBS player, but os.sleep will often sleep more than I tell it to.

http://sprunge.us/WiCQ (left is time slept, right is sleep asked)

Kubuxu commented 8 years ago

It is not often (I can see only few) and those are one tick differences, not much can be done in that regard, same things happens with redstone.

SoniEx2 commented 8 years ago

It works fine in CC. Also if I loop 0.01 sleeps or take 0.01 from how much I wanna sleep it always sleeps right.

if i>0 then os.sleep(i/20) end = http://sprunge.us/WiCQ if i>0 then os.sleep(i/20-0.01) end = http://sprunge.us/VeIE (this one still saves power tho) for _=1,i do os.sleep(0.01) end = http://sprunge.us/OQQE (not power efficient)

SoniEx2 commented 8 years ago

Does OC decrement the actual time difference between update() calls, instead of a fixed 0.05 per call? That seems to be what's happening...

SoniEx2 commented 8 years ago

Oh, see also dan200/ComputerCraft#10

Kubuxu commented 8 years ago

As OC uses thread pool it isn't that simple as in case of CC.

SoniEx2 commented 8 years ago

Well I just noticed os.sleep is actually implemented in OpenOS... It's simpler than I thought, just needs a bit of rounding...

SoniEx2 commented 8 years ago

Hmm nvm, it's not, it relies on event.pull, which's inconsistent even when you have only a single computer running...

gjgfuj commented 8 years ago

Yes, OC doesn't have accurate timing if you yield at all. Don't ever rely on it.

On Tue, 23 Feb 2016 12:05 am Soni L. notifications@github.com wrote:

Hmm nvm, it's not, it relies on event.pull, which's inconsistent even when you have only a single computer running...

— Reply to this email directly or view it on GitHub https://github.com/MightyPirates/OpenComputers/issues/1667#issuecomment-187162254 .

Vexatos commented 8 years ago

The problem is that you can't not yield. You must yield and that makes timed programs impossible.

gjgfuj commented 8 years ago

Exactly.

On Tue, 23 Feb 2016 1:15 am Vexatos notifications@github.com wrote:

The problem is that you can't not yield.

— Reply to this email directly or view it on GitHub https://github.com/MightyPirates/OpenComputers/issues/1667#issuecomment-187195840 .

SoniEx2 commented 8 years ago

I don't need it to be accurate if I'm running 100 computers. I do need it to be accurate if I'm running 1 computer. There's a bit of a huge difference there.

MaHuJa commented 8 years ago

Basically, os.sleep(x), and its equivalent primitives in every threaded system ever*, says "do not wake me up for AT LEAST x". Or to be more precise, "whenever you want to schedule me again, if x time has not (fully) passed, don't."

You must yield and that makes timed programs impossible.

Assuming a world that has no other computers, and not much else, I expect it to try scheduling the computer for running at least once every world tick. If not, that's our bug right there. The only case where I see it legitimately skipping scheduling attempts is when other computers have depleted the available time for this tick.

Assuming world ticks to be the granularity to which we want to time it, it follows from the first assumption that this is possible, if not straightforward.

I did not see in your logs (only scanned them by eye) any case of it sleeping more than .05 too long (1 tick). So what happened is basically this: 0s : we run our code 0.001s : we ask it to sleep for 2 seconds. 0.05s : it tries to schedule us, but we're supposed to sleep until 2.001 0.10s : ditto ... 2.00s : This is when you wanted, but 2 < 2.001 so it's not woken up. 2.05s : Now it wakes up, but it's a tick late. That is, in a nutshell, "tick alignment". This is greatly amplified from real computers by only having 20 wakeup chances in a second.

The obvious workaround is, as you noticed, subtracting a little bit. local realsleep=os.sleep; function os.sleep(time) return realsleep(time-0.025) end

Do we want to discuss the pros and cons of doing this by default? One con is that running into this is a learning opportunity, exactly because it's amplified enough that you notice it.

It works fine in CC.

Unless my memory is faulty, CC sleeps have no connection to real time whatsoever. In contrast, if you say os.sleep(10), OC will continue 10 real seconds later, even if the server is running at 5 tps. It'll just be after 50 ticks worth of world processing rather than 200.

This makes your CC program very vulnerable to tps drops, and there's nothing you can do about it. On the other hand, that sleep will never happen at 0.001 seconds, because CC visible time does not move forward while CC code is running.

The CC forum thread linked from the CC bug says their bug came from the update subtracting 0.5, which does not have an exact binary representation, thus they really subtracted something like 0.499... so therefore there would be around 0.000000001 seconds left to sleep, thus it slept a tick too long.

But for the nbs player case, a sleep_until(starttime + duration * i) may be a better fit. And I don't see why it could not be implemented in terms of os.sleep and thus by the user himself. (Beware chunk unloading.) Because depending on that starttime, it would be either on-time or all 1-tick late. It's when they're mixed the nbs player will have a problem.

As an aside, you may also run into a problem calling functions that need to interact with the game world, needing to synchronize with the main mc server thread or some such. This may be a once-a-tick opportunity, and thus such calls may block until the next tick rolls around. I'm not entirely clear on the details there.

See also https://youtu.be/6YozL_Mg3Tk?t=6m42s

iamgreaser commented 8 years ago

See also https://youtu.be/6YozL_Mg3Tk?t=6m42s

which used computer.uptime() to sync.

Don't use sleep() for your timing method. You need to read a clock to sync things, too. This applies to everything ever. In other words, sleep != setitimer.

SoniEx2 commented 7 years ago

OC's sleep works based on true time deltas (aka real seconds), while CC's sleep works based on approximated time deltas (it assumes 0.05s per tick).

OC and OC programs are pretty much immune to tickrate changer, while CC and CC programs will happily run at slower or faster tickrates.

If you really want your program to reflect tickrate changes, I'd recommend sleep(0) in a loop. It just sucks that this is not the default.

payonel commented 7 years ago

I don't see anything to fix here - event.pull will yield for the specified time in real seconds at a minimum, it may even yield longer. This is by design. It would be a bug if we were not yielding long enough. Even sleep(0) is calling event.pull(0) which is calling computer.pullSignal(0). While we are resuming the machine coroutine at the next chance we get, we're not making guarantees that it'll be the very next tick. We're also not guaranteeing that the tick will be 1/20th of a second.

SoniEx2 commented 7 years ago

@payonel Makes it hard to play noteblock songs :/

payonel commented 7 years ago

OpenFM plays a stream, Computronics can play files saved to tape in the proper format

SoniEx2 commented 7 years ago

@payonel And neither is a music tracker format.

payonel commented 7 years ago

@SoniEx2 hard yes, but not impossible. you can synchronize the timing of the notes using computer.uptime, but you can't rely on os.sleep for this precision.

SoniEx2 commented 7 years ago

@payonel you can in computercraft. and it'd be fairly trivial to change the OC code for it.

It'd make running CC programs in OC much easier. The CC emulator for OC sucks because of things like this.