Move Jellyfin.Networking

[barronpm]

Changes

• Moves Jellyfin.Networking to src
• Moves networking code from Emby.Server.Implementations to Jellyfin.Networking
• Fixes some warnings in the aforementioned networking code
• Uses file-scoped namespaces for moved code

Changes are probably best reviewed commit-by-commit

[JPVenson]

As its a Task not a ValueTask its fine to await them anyway even if already completed. So either make AttemptConnection return a ValueTask (which would be the correct usage for it) and/or await it here anyway.

[barronpm]

All of this code is being moved w/ as few changes as possible to make it compile, so any further changes aren't relevant for this PR.

[cvium]

There's absolutely no point in using await. The task is complete. The only reason GetAwaiter.GetResult is used over .Result is error handling (.Result throws AggregateException)

[JPVenson]

await should always be considered for 3 reasons:

• Change of implementation. When the underlying implementation changes this could be a real task
• Again, that why a ValueTask should be used here. ValueTask represent exactly this behavior.
• Syntactic correctness. If we are in an async method, we should always await tasks.

[cvium]

I prefer to do what Stephen Toub says: https://github.com/dotnet/corefx/pull/29792/files#r189415885. He seems like a smart enough guy :)

[JPVenson]

Understandable, i read the whole discussion before starting this :-D

But i would argue that even readability in our, way less critical scenario is more important then 5 bytes and 1ms additional time.
Tbo i dont know why he did not even considered using ValueTask

[cvium]

Can't use a ValueTask in Task.WhenAny though, so how would you wait for a result but only for 200ms?

[JPVenson]

As its just one task you want to wait for 200ms, i would just use await tryConnectAsyncIPv6.WaitAsync(200).ConfigureAwait

[cvium]

There is no WaitAsync for ValueTask :)

[JPVenson]

Oh really? Then i have to apologise :-)

[JPVenson]

I would prefer to use Task.Factory.StartNew(..., TaskCreationOptions.LongRunning) to not clog up the scheduler

[cvium]

But it's not long running. Whenever you hit an actual await, the continuation occurs on a thread pool thread. You need to create your own task scheduler to avoid this.

[JPVenson]

That is what TaskCreationOptions.LongRunning does. It does not schedule the Task on the local Thread pool thread.

[cvium]

Only the first part of the code will be on that longrunning thread. It's very hard to force something to run on a dedicated thread when async is involved. Thankfully, it's very easy to verify :)

            var task = Task.Factory.StartNew(async () =>
            {
                while (true)
                {
                    Console.WriteLine($"Before await. Is thread pool thread? {Thread.CurrentThread.IsThreadPoolThread}");
                    await Task.Delay(100);
                    Console.WriteLine($"After await. Is thread pool thread? {Thread.CurrentThread.IsThreadPoolThread}");
                }
            }, TaskCreationOptions.LongRunning);
            Console.ReadKey();

Output:

Before await. Is thread pool thread? False
After await. Is thread pool thread? True
Before await. Is thread pool thread? True
After await. Is thread pool thread? True
Before await. Is thread pool thread? True
After await. Is thread pool thread? True

[JPVenson]

^ that is correct. LongRunning just prevents oversubscription on the actual scheduler. The scheduler does have a limited amount of Threads its allowed to manage and that includes continuations.

Also, LongRunning hints at the scheduler that it may or may not schedule the task on its own dedicated thread just from the start. That also depends on the environment you are running in.

[cvium]

Creating a long running thread when the most immediate action is to await something is a (relatively) huge waste of resources.

[JPVenson]

No, that is not want i meant. Sry.

Tasks are managed by a Scheduler, and the continueations of those Tasks are handled by the same Scheduler. The default one just puts them into the default ThreadPool. The Default scheduler is responsible for all tasks and this might cause oversubscriptions to the default ThreadPool which can hurt the actual responsiveness of the app. That means if you have an operation that runs for long periods of time, its recommended to use that flag for the Scheduler so it does not do that and schedules it outside the default pool.

[cvium]

That is correct, but when you use async-await it's not long running, and as you can see by the sample code I provided, the continuation is now bound to the Thread pool. Thus, the Long Running Task is a useless allocation of resources if there's no CPU bound task.

[JPVenson]

Ideally _receiveBuffer should not live on the Heap as it has no use there. What is the idea behind putting it there?

[cvium]

It's a standard array. It always lives on the heap. Whether or not it should be a pinned array or be pulled from the array pool, I don't know.

[JPVenson]

Best would be to get an encoder once and keep it.

[cvium]

What do you mean by this? Encoding.UTF8 is static.

[JPVenson]

You can get an encoder with Encoding.UTF8.GetEncoder() and keep that. Is a bit more efficient.

[cvium]

Seems identical to me

// * Summary *

BenchmarkDotNet v0.13.10, Windows 10 (10.0.19045.3693/22H2/2022Update)
AMD Ryzen 5 3600, 1 CPU, 12 logical and 6 physical cores
.NET SDK 8.0.100
  [Host]     : .NET 8.0.0 (8.0.23.53103), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.0 (8.0.23.53103), X64 RyuJIT AVX2


| Method     | Mean     | Error    | StdDev   | Gen0   | Allocated |
|----------- |---------:|---------:|---------:|-------:|----------:|
| Static     | 18.77 ns | 0.376 ns | 0.369 ns | 0.0057 |      48 B |
| GetDecoder | 19.61 ns | 0.418 ns | 0.899 ns | 0.0057 |      48 B |

[MemoryDiagnoser]
public class EncoderBenchmark
{
    private static readonly Decoder _decoder = Encoding.UTF8.GetDecoder();
    private static readonly byte[] _array = Encoding.UTF8.GetBytes("Hello World");

    [Benchmark]
    public void Static()
    {
        _ = Encoding.UTF8.GetString(_array);
    }

    [Benchmark]
    public void GetDecoder()
    {
        Span<char> r = stackalloc char[_array.Length];
        _decoder.GetChars(_array, r, true);
        _ = new string(r);
    }
}