On Fri, 01 May 2009, Pritpal Bedi wrote:
Hi,
> In GTWVT | GTWVG we have this function:
> static PHB_GTWVT hb_gt_wvt_Find( HWND hWnd )
> {
> int iCount = s_wvtCount, iPos = 0;
> PHB_GTWVT pWVT = NULL;
>
> HB_WVT_LOCK
> while( iCount && iPos < WVT_MAX_WINDOWS )
> {
> if( s_wvtWindows[ iPos ] )
> {
> if( s_wvtWindows[ iPos ]->hWnd == hWnd )
> {
> pWVT = s_wvtWindows[ iPos ];
> break;
> }
> --iCount;
> }
> ++iPos;
> }
> HB_WVT_UNLOCK
>
> return pWVT;
> }
> As this function just reads the static array,
> can we avoid HB_WVT_LOCK | UNLOCK calls ?
This static array can be modified by hb_gt_wvt_Free(). Without
protection the above loop can GPF. F.e.:
while( iCount && iPos < WVT_MAX_WINDOWS )
{
if( s_wvtWindows[ iPos ] )
{
// s_wvtWindows[ iPos ] was not NULL but
// now other thread closed the window at iPos setting
// s_wvtWindows[ iPos ] to NULL
if( s_wvtWindows[ iPos ]->hWnd == hWnd )
// so in above line we have GPF
> It sppeds up the things considerably.
> But I do not know the fatalitites in real life.
Have you made some real speed tests?
Hwat is the difference?
Have you used single or multi CPU machine?
In fact this loop is not efficient but the most expensive should
be the scanning process not lock/unlock operations.
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.
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.
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.
best regards,
Przemek
_______________________________________________
Harbour mailing list
[email protected]
http://lists.harbour-project.org/mailman/listinfo/harbour
