Hello Przemek
First thanks for detailed analysis.
> This static array can be modified by hb_gt_wvt_Free(). Without
> protection the above loop can GPF. F.e.:
>
I knew this can be the case so that is why I posted this
message to gather your other views.
> Have you made some real speed tests?
> Hwat is the difference?
> Have you used single or multi CPU machine?
>
No, I did not made any tests. Infact it is not practical
to measure speed by any means. But I could sense it can
make considerable difference as Windows issues a tons
of messages and hence in hb_gt_wvt_WndProc() this
loop is scanned hundreds of time per second. And
multiply this scenario with number of windows open,
and you can imagine the facts.
> In fact this loop is not efficient but the most expensive should
> be the scanning process not lock/unlock operations.
>
True.
> It can be resolved in different way. We can store pointer to PHB_GTWVT
> structure inside window data, f.e. at GWL_USERDATA:
>
> SetWindowLongPtr( pWVT->hWnd, GWL_USERDATA, ( LONG_PTR ) pWVT );
>
> In such case it's possible to extract PHB_GTWVT by:
>
> PHB_GTWVT pWVT = GetWindowLongPtr( hWnd, GWL_USERDATA );
>
> It's highly possible that it will be faster but you should check it.
> I do not know what internally GetWindowLongPtr() does and how expnesive
> it can be.
> Of course it also means that GWL_USERDATA cannot be used to any other
> things and should be documented that it belongs GT internal structures.
>
By far this is the most efficient way on paper,
but I do not know the real cost of
PHB_GTWVT pWVT = GetWindowLongPtr( hWnd, GWL_USERDATA );
But for sure itwill be much lighter than mutexing and scanning the whole
array.
> Different GUI libraries usually gives some support to bounf with given
> window some cargo value which can be used in the same way as GWL_USERDATA
> in the above example.
>
Yes. Alternatively we can used
SetProp( hWnd, "pWVT", ( LONG_PTR ) pWVT ) | GetProp( hWnd, "pWVT" ).
Infact in GTWVG I am using this mechanism to store CallBack pointers.
But for GTWVT and GTWVG
Set|GetWindowLongPtr( hWnd, GWL_USERDATA );
appears to be more optimized.
> There is also yeat another solution. We can try to change hb_gt_wvt_Find()
> to make it safe for concurent modifications, f.e. we can try to make:
>
> static PHB_GTWVT hb_gt_wvt_Find( HWND hWnd )
> {
> int iPos = 0;
> PHB_GTWVT pWVT;
> while( iPos < WVT_MAX_WINDOWS )
> {
> pWVT = s_wvtWindows[ iPos ];
> if( pWVT )
> {
> if( pWVT->hWnd == hWnd )
> return pWVT;
> }
> ++iPos;
> }
> return NULL;
> }
>
> In such case we should replace possible accessing NULL pointer with
> acessing some memory which may not belong to the process. Probably
> it will reduce the chance for GPF but it's still not 100% safe solution.
> To make it fully safe we should change the structure of s_wvtWindows
> array. Instead of storing pointer to PHB_GTWVT structures we can store
> pairs: pWVT and hWnd. To reach such effect we can change s_wvtWindows or
> we can add new static array:
> static HWND s_wvtWindowsHandles[ WVT_MAX_WINDOWS ];
> which will be updated with s_wvtWindows in hb_gt_wvt_Alloc():
>
> s_wvtWindows[ iPos ] = pWVT;
> s_wvtWindowsHandles[ iPos ] == pWVT->hWnd;
>
> and in hb_gt_wvt_Free():
>
> s_wvtWindowsHandles[ pWVT->iHandle ] == NULL;
> s_wvtWindows[ pWVT->iHandle ] = NULL;
>
> of course the locks in hb_gt_wvt_Alloc() and hb_gt_wvt_Free() have to
> still exists. Then hb_gt_wvt_Find() can look:
>
> static PHB_GTWVT hb_gt_wvt_Find( HWND hWnd )
> {
> int iPos = 0;
> while( iPos < WVT_MAX_WINDOWS )
> {
> if( s_wvtWindowsHandles[ iPos ] == hWnd )
> return s_wvtWindows[ iPos ];
> }
> return NULL;
> }
>
> and such version is MT safe. You can use it and if you think it causes
> noticeable speed difference then you can update SVN (of course please
> verify my possible typos in the above peaces of code)
> Anyhow it still makes WVT_MAX_WINDOWS iterations what may consume some
> noticeable peace of time. The linear scan can be replaces by binary search
> but in such case we will have to restore locking inside hb_gt_wvt_Find()
> because hb_gt_wvt_Alloc() and hb_gt_wvt_Free() have to recreate good order
> and such operation has to be mutual exclusive to hb_gt_wvt_Find(). What is
> the most expensive: locking or scanning?
> To answer you will have to make some tests with big number of open
> windows.
> You can also try to introduce some type of hashing with bucket sorting.
> In such case it should be possible to keep hb_gt_wvt_Find() without any
> locks. But it can improve the performance only if you can create some
> efficient function which maps hWnd to some window handle set numbers
> (bucket number). I do not know how windows allocate handles and if it's
> easy or not to create such function which can also give good unique
> results.
>
The other options are more or less have the same scanning panelty.
Imagine a scenario:
1) Open 20 windows.
2) Close first 19 windows, keep open 20th window.
3) Open another window, close, open, close.
4) There is no reduction in scan loop, it will always scan 20th iteration
for
any message sent to 20th window.
I will make some experiments with Set|GetWindowLong() | Set|GetProp()
methods and will report which is working correctly.
Regards
Pritpal Bedi
--
View this message in context:
http://www.nabble.com/GTWVT-%7C-WVG---Question-about-Mutex-tp23329155p23333364.html
Sent from the Harbour - Dev mailing list archive at Nabble.com.
_______________________________________________
Harbour mailing list
[email protected]
http://lists.harbour-project.org/mailman/listinfo/harbour
