Re: [PATCH] taskmgr: Implement process page sorting.

[Nikolay Sivov]

On 4/6/21 11:09 AM, Haoyang Chen wrote:
> 在 2021/4/6 下午4:01, Haoyang Chen 写道:
> > From: Nikolay Sivov 
> > Subject: Re: [PATCH] taskmgr: Implement process page sorting.
> > Message-Id: 
> > Date: Tue, 6 Apr 2021 08:42:36 +0300
> > In-Reply-To: 
> > References:
> >
> > On 4/6/21 4:48 AM, Haoyang Chen wrote:
> > > Signed-off-by: Haoyang Chen
> > >
> > > programs/taskmgr/perfdata.c | 76 +++++++++++++++++++++++++++++++++++++
> > >  programs/taskmgr/perfdata.h |  1 +
> > >  programs/taskmgr/procpage.c | 10 ++---
> > >  3 files changed, 81 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/programs/taskmgr/perfdata.c b/programs/taskmgr/perfdata.c
> > > index 2cbd6b01669..52a08da4d88 100644
> > > --- a/programs/taskmgr/perfdata.c
> > > +++ b/programs/taskmgr/perfdata.c
> > > @@ -833,3 +833,79 @@ ULONG PerfDataGetTotalThreadCount(void)
> > >
> > > return ThreadCount;
> > >  }
> > > +/* sort code copied from DPA */
> >
> > Why do you need to copy it?
>
> The original DPA_QuickSort sort is for LPVOID, now it is used for
> PPERFDATA.

It's a helper from another module, there is a qsort() for that.
> > > +static BOOL bSortAscending = TRUE;
> >
> > This does not have to be global.
>
> Because both processpagecomparefunction and perdatasort use it.
> > > +static INT CALLBACK ProcessPageCompareFunc(LPVOID first, LPVOID
> > > second, LPARAM lParam)
> > > +{
> > > +    PPERFDATA pPerfData1 = NULL;
> > > +    PPERFDATA pPerfData2 = NULL;
> > > +    INT iSubItem = lParam;
> > > -
> > >
> > > +    if (bSortAscending)
> > > +    {
> > > +        pPerfData1 = (PPERFDATA) first;
> > > +        pPerfData2 = (PPERFDATA) second;
> > > +    }
> > > +    else
> > > +    {
> > > +        pPerfData2 = (PPERFDATA) first;
> > > +        pPerfData1 = (PPERFDATA) second;
> > > +    }
> > > -
> > >
> > > +    switch (iSubItem)
> > > +    {
> > > +        case 0:
> > > +            return lstrcmpW(pPerfData1->ImageName,
> > > pPerfData2->ImageName);
> > > +        case 1:
> > > +            return pPerfData1->ProcessId > pPerfData2->ProcessId;
> > > +        case 2:
> > > +            return pPerfData1->CPUUsage > pPerfData2->CPUUsage;
> > > +        case 3:
> > > +            return pPerfData1->CPUTime.QuadPart >
> > > pPerfData2->CPUTime.QuadPart;
> > > +        case 4:
> > > +            return pPerfData1->vmCounters.VirtualSize >
> > > pPerfData2->vmCounters.VirtualSize;
> > > +    }
> > > +    return 0;
> > > +}
> >
> > We have 25 columns in total, with symbolic names defined for them.
>
> ok, I will send v2,  thanks.
> > > -
> > >
> > > +static VOID DPA_QuickSort (PPERFDATA lpPtrs, INT l, INT r,
> > > +                           PFNDPACOMPARE pfnCompare, LPARAM lParam)
> > > +{
> > > +    INT m;
> > > +    PERFDATA  t;
> > > -
> > >
> > > +    if (l==r)    /* one element is always sorted */
> > > +        return;
> > > +    if (r<l)     /* oops, got it in the wrong order */
> > > +    {
> > > +        DPA_QuickSort(lpPtrs, r, l, pfnCompare, lParam);
> > > +        return;
> > > +    }
> > > +    m = (l+r)/2; /* divide by two */
> > > +    DPA_QuickSort(lpPtrs, l, m, pfnCompare, lParam);
> > > +    DPA_QuickSort(lpPtrs, m+1, r, pfnCompare, lParam);
> > > -
> > >
> > > +    /* join the two sides */
> > > +    while( (l<=m) && (m<r) )
> > > +    {
> > > - if(pfnCompare(&lpPtrs[l],&lpPtrs[m+1],lParam)>0)
> > >
> > > +        {
> > > +            t = lpPtrs[m+1];
> > > - memmove(&lpPtrs[l+1],&lpPtrs[l],(m-l+1)*sizeof(lpPtrs[l]));
> > >
> > > +            lpPtrs[l] = t;
> > > -
> > >
> > > +            m++;
> > > +        }
> > > +        l++;
> > > +    }
> > > +}
> > > -
> > >
> > > +VOID PerDataSort(UINT iSubItem)
> > > +{
> > > +    EnterCriticalSection(&PerfDataCriticalSection);
> > > -
> > >
> > > +    bSortAscending = !bSortAscending;
> > > +    DPA_QuickSort(pPerfData, 0, ProcessCount - 1,
> > > ProcessPageCompareFunc, iSubItem);
> > > -
> > >
> > > +    LeaveCriticalSection(&PerfDataCriticalSection);
> > > +}
> > > \ No newline at end of file
> > > diff --git a/programs/taskmgr/perfdata.h b/programs/taskmgr/perfdata.h
> > > index 615e7db2a64..f39b03c503d 100644
> > > --- a/programs/taskmgr/perfdata.h
> > > +++ b/programs/taskmgr/perfdata.h
> > > @@ -110,4 +110,5 @@ ULONG    PerfDataGetSystemHandleCount(void);
> > >
> > > ULONG    PerfDataGetTotalThreadCount(void);
> > >
> > > +VOID    PerDataSort(UINT iSubItem);
> > >  #endif /* __PERFDATA_H */
> > > diff --git a/programs/taskmgr/procpage.c b/programs/taskmgr/procpage.c
> > > index 5ea2e6b044f..f8dc0a0e89d 100644
> > > --- a/programs/taskmgr/procpage.c
> > > +++ b/programs/taskmgr/procpage.c
> > > @@ -155,12 +155,14 @@ static void ProcessPageOnNotify(LPARAM lParam)
> > >      ULONG              ColumnIndex;
> > >      IO_COUNTERS        iocounters;
> > >      TIME               time;
> > > +    NMLISTVIEW*        pnmListView;
> > >      static const WCHAR wszFmtD[] = {'%','d',0};
> > >      static const WCHAR wszFmt02D[] = {'%','0','2','d',0};
> > >      static const WCHAR wszUnitK[] = {' ','K',0};
> > >
> > > pnmh = (LPNMHDR) lParam;
> > >      pnmdi = (NMLVDISPINFOW*) lParam;
> > > +    pnmListView = (NM_LISTVIEW*)lParam;
> > >
> > > if (pnmh->hwndFrom == hProcessPageListCtrl)
> > >      {
> > > @@ -343,12 +345,8 @@ static void ProcessPageOnNotify(LPARAM lParam)
> > >          {
> > >          case HDN_ITEMCLICKW:
> > >
> > > -            /*
> > > -             * FIXME: Fix the column sorting
> > > -             *
> > > -             *ListView_SortItems(hApplicationPageListCtrl,
> > > ApplicationPageCompareFunc, NULL);
> > > -             *bSortAscending = !bSortAscending;
> > > -             */
> > > +             PerDataSort(pnmListView->iItem);
> > > +             InvalidateRect(hProcessPageListCtrl, NULL, TRUE);
> > >
> > > break;
> >
> > HDN_ITEMCLICK does not use NMLISTVIEW structure.
>
> ok, I will send v2,  thanks.

Actually, why are you only sorting on header clicks?