toddams / RazorLight

Template engine based on Microsoft's Razor parsing engine for .NET Core
Apache License 2.0
1.52k stars 259 forks source link

avoid locking threads on compile #425

Closed SimonCropp closed 3 years ago

toddams commented 3 years ago

Thank you for your contribution. Can you please provide some explanations to your further MR, like why this change is introduced and what issue it resolves. I am also curious about this one, why SemaphoreSlim instead of lock?

SimonCropp commented 3 years ago

SemaphoreSlim allows async waiting. which doesnt block a thread

jzabroski commented 3 years ago

@toddams Is it causing any problems? It seems useful to me to be able to avoid blocking congestion? Granted, I don't have any uses with millions of requests a second that might be worth writing a benchmark test for.

SimonCropp commented 3 years ago

@jzabroski i didnt do this for perf reasons. but to remove the .GetAwaiter().GetResult(); which is a common cause of deadlocks

jzabroski commented 3 years ago

@SimonCropp That is what I am referring to. My understanding is the deadlock only triggers when you exhaust all threads and the threads can't finish because the "buffer" in this case is the thread pool size. Am I wrong? Thank you.

jzabroski commented 3 years ago

Currently, our RaceCondition Tests in the build run in ~20 seconds but I think we only create 1,000 tasks, which would not stress the thread pool I believe. I wonder if the test (or another test) should call ThreadPool.GetMaxThreads and create a scenario where we test the number of concurrent tasks > max threads.