Execute tasks scheduled at the same time in order on EmbeddedEventLoop

[glbrntt]

Motivation:

EmbeddedEventLoop executes scheduled tasks in their run-time order.
If multiple tasks are scheduled for the same time then the order in
which they are eventually executed is undefined.

Since execute defers to scheduling a task for "now" it is possible for
tasks to be executed out of order if run() is not called between each
execute.

In "real" event loops this isn't an issue because the system time must
advance between each call to execute.

Modifications:

• Add a 'insertionOrder' to EmbeddedScheduledTask; use it to break
  ties when comparing two tasks with the same run time
• Add a task counter to EmbeddedEventLoop which is used to provide an
  insertion order when scheduling tasks.

Result:

• EmbeddedEventLoop will execute tasks in order

[Lukasa]

In "real" event loops this isn't an issue because the system time must
advance between each call to execute.

I mean...this is not actually true. Ultimately we delegate to DispatchTime.now, which on macOS delegates to mach_absolute_time and on Linux delegates to clock_gettime with CLOCK_MONOTONIC. Leaving aside the Mach case, while CLOCK_MONOTONIC is a monotonically incrementing clock, it does not say how long it may stay at the same value. For example, CLOCK_MONOTONIC quite famously does not count forward for time spent while the system is suspended. Additionally, system clocks do have a minimum resolution, and it's completely plausible for that minimum resolution to be substantially larger than the amount of time in which processors execute instructions.

This is not inherently a reason to refuse to make this change, but it's worth bearing in mind that there is absolutely no promise that tasks execute in any particular order.

As a related note, this patch actually affects all scheduled tasks. This is a strict change from the behaviour of SelectableEventLoop, where if two tasks are enqueued that have the same deadline they will be run in a random order. Again, this divergence is not critical as we don't actually make any guarantees here, but it's worth noting.

@weissi and @normanmaurer, where do you land here? Is it valuable to have EmbeddedEventLoop reflect the same ordering uncertainty that SelectableEventLoop has, even though on EmbeddedEventLoop vastly more tasks are enqueued at the exact same time?

[weissi]

@Lukasa what you're saying is definitely correct. We could hit this issue in the real world too. I'm happy with merging this and filing an issue that we should also add an insertion counter atomic and use that as a tie breaker if we have two tasks with equal ready times.

[Lukasa]

Currently with our Big Dumb Lock it doesn't even need to be atomic: we take a look to enqueue tasks irregardless, so we can always just use that lock to guard the counter.

[glbrntt]

In "real" event loops this isn't an issue because the system time must
advance between each call to execute.

I mean...this is not actually true. Ultimately we delegate to DispatchTime.now, which on macOS delegates to mach_absolute_time and on Linux delegates to clock_gettime with CLOCK_MONOTONIC. Leaving aside the Mach case, while CLOCK_MONOTONIC is a monotonically incrementing clock, it does not say how long it may stay at the same value. For example, CLOCK_MONOTONIC quite famously does not count forward for time spent while the system is suspended. Additionally, system clocks do have a minimum resolution, and it's completely plausible for that minimum resolution to be substantially larger than the amount of time in which processors execute instructions.

Interesting; thanks for the details.

This is not inherently a reason to refuse to make this change, but it's worth bearing in mind that there is absolutely no promise that tasks execute in any particular order.

If we don't accept this change then we should state this explicitly in the documentation.

[weissi]

@glbrntt I'd say: Let's take this & file a bug on NIO

[Lukasa]

Works for me.

[weissi]

hit #1545

[weissi]

@swift-nio-bot test this please

[glbrntt]

Ping @weissi for review

[weissi]

Thank you! LGTM, sorry for the delay, thought I had done this one already 😭