Why would one use TLazMonitor over TCriticalSection?

[PascalB]

I am currently investigating an instance where TLazMonitor massively underperforms on Windows, but works fine on Mac and Linux.

Before creating an issue and proposing a patch on GitLab, I need more information.

I came across this issue while using TLazThreadedQueue from the unit LazCollections. Pushing and popping items worked very slowly.

I discovered that TLazThreadedQueue uses a TLazMonitor object internally and that TLazMonitor does busy-sleeping through calls to sleep(1) and sleep(0). I believe the calls to sleep(1) are problematic since the default timer resolution, on Windows, is around 16 ms.

I can work around this issue by calling timeBeginPeriod/timeEndPeriod and passing 1 ms as the argument. However, this fix is unacceptable. Depending on the version of Windows, timeBeginPeriod/timeEndPeriod may affect the whole system or the whole application, leading to unwanted side effects. In some instances, Windows does not even guarantee that the higher resolution will be honored.

By replacing the TLazMonitor object with a TCriticalSection, TLazThreadedQueue becomes much faster. I have tested this change on Windows, Mac and Linux, and it seems to work just fine.

I have tried to understand why TLazMonitor (which is a child of TCriticalSection) is used instead of TCriticalSection directly, but could not find any answers. Moreover, TLazMonitor is barely used in Lazarus' codebase and barely mentioned anywhere on the Internet.

Does someone know why one might want to use TLazMonitor over TCriticalSection?

[jamie]

Maybe a long-forgotten relic?

Jamie

[Martin_fr]

I don't really know... But from looking at it.

It tries to avoid the call to "EnterCriticalSection(CriticalSection);" => if that critical section is already entered.

That is, if that call would mean the thread would have to wait, and therefore to enter some form of waiting/sleeping state from which it has to be woken up later.

So, it seems that (at least on the OS on which that was tested / for which this was designed / and at least at that time), entering such a sleeping state was slower than trying a spin lock.

But maybe those timings are different on diff OS? And diff versions thereof...



I don't know why sleep(1) comes before sleep(0)
At least not on win, don't know if other OS have other rules.
https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-sleep

E.g. on a single core OS, a sleep(0) is needed to allow the other thread to give up the CS.

But the entire code only makes sense if there are several cores, and threads are running on different cores.


On the other hand, if one does highly specialized thread locking, then it should be chosen on the task at hand. => if the locked code is very very quick, then a spinlock should always be enough. (Especially on multicore).

Then no CS would be needed at all. Just an interlockexchanged flag, waited for in a spin.

[PascalB]

I don't really know... But from looking at it.

It tries to avoid the call to "EnterCriticalSection(CriticalSection);" => if that critical section is already entered.

That is, if that call would mean the thread would have to wait, and therefore to enter some form of waiting/sleeping state from which it has to be woken up later.

So, it seems that (at least on the OS on which that was tested / for which this was designed / and at least at that time), entering such a sleeping state was slower than trying a spin lock.

But maybe those timings are different on diff OS? And diff versions thereof...



I don't know why sleep(1) comes before sleep(0)
At least not on win, don't know if other OS have other rules.
https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-sleep

E.g. on a single core OS, a sleep(0) is needed to allow the other thread to give up the CS.

But the entire code only makes sense if there are several cores, and threads are running on different cores.


On the other hand, if one does highly specialized thread locking, then it should be chosen on the task at hand. => if the locked code is very very quick, then a spinlock should always be enough. (Especially on multicore).

Then no CS would be needed at all. Just an interlockexchanged flag, waited for in a spin.

Okay, thanks for the pointers. I'll investigate further.

If I go forward with a patch, should I aim to fix TLazMonitor (so that it performs well on Windows) or TLazThreadedQueue (so that it doesn't use TLazMonitor)? Or could it go either way depending on my findings and my judgment call? I have only contributed one patch so far, so I'm still new to this process.