Skip to main content
Code Review

Return to Revisions

15 of 15
replaced http://stackoverflow.com/ with https://stackoverflow.com/

Performance

[..] winforms is not powerful so it takes 1-2 seconds to refresh the content [..]

It's not WinForms because it's actually very fast and I have never had any issues with it. There is a 99.99% chance that the code is inefficient so let's have a look.

Expression.Compile()

This is what slows the application down and where the bottle-neck is hidden and where the Expressions bite.

private void SortProcesses<T>(Expression<Func<Process, T>> lambda)
 where T : IComparable
{
 orderedProcessesInfo.RemoveAll(p => p.Value.HasExited);
 orderedProcessesInfo.Sort(
 (process1, process2) =>
 lambda.Compile()
 .Invoke(process1.Value).CompareTo(lambda.Compile()
 .Invoke(process2.Value)));
 if (isAscendingOrder)
 {
 orderedProcessesInfo.Reverse();
 }
 processesInfo = orderedProcessesInfo.ToDictionary(pair => pair.Key, pair => pair.Value);
 UpdateProcessList();
}

There are two (!) Compiles. They are very expensive and there is no need for them and the Expression because there is nothing that dynamically changes. You have always a Process and a value to get and to compare. Use just the Func:

private void SortProcesses<T>(Func<Process, T> getProperty)
 where T : IComparable
{
 // ...
 orderedProcessesInfo.Sort((process1, process2) =>
 getProperty(process1.Value)
 .CompareTo(getProperty(process2.Value))
 );
 // ...
}

This is a sort method and it needs to run fast. If it doesn't, you'll notice it right away.


Here's a link to @Eric's Lippert answer on Stack Overflow explaining what the Compile method does: What does Lambda Expression Compile() method do?.

And one more link to another answer (by someone else) on Stack Overflow comparing execution times of various method calls: Performance of Expression.Compile vs Lambda, direct vs virtual calls


Another thing that I don't like about it is this

orderedProcessesInfo.Reverse()

The Sort should already produce the right order. The above line looks like the sorting wouldn't work correctly and you need this workaround to fix it instead of fixing the sort function.


ListView.Items.Add()

The second method that most likely makes you think WinForms wouldn't be performing well is this one:

public void UpdateProcessList()

You call here

ListViewProcesses.Clear();

[..] this method removes all items and columns from the ListView control

only to immediately recreate the columns with

ListViewProcesses.Columns.Add(..);

Do you really want to do it each time you refresh the list? You have already created the list-view once. I guess what you really wanted to do is to just remove all the items with ListView.Items.Clear().

This and adding the many rows to the list in a loop without suspending it really hurt the performance because the list-view keeps refreshing after each change.

foreach (var processInfo in processesInfo)
{ 
 // ..
 ListViewProcesses.Items.Add(..);
 // ..
}

Consider using the BeginUpdate and EndUpdate methods and adding the rows inbetween or better, use the AddRange method:

The preferred way to add multiple items to a ListView is to use the AddRange method of the ListView.ListViewItemCollection (accessed through the Items property of the ListView). This enables you to add an array of items to the list in a single operation. However, if you want to add items one at a time using the Add method of the ListView.ListViewItemCollection class, you can use the BeginUpdate method to prevent the control from repainting the ListView every time that an item is added. When you have completed the task of adding items to the control, call the EndUpdate method to enable the ListView to repaint. This way of adding items can prevent flickered drawing of the ListView when lots of items are being added to the control.

ListView.BeginUpdate Method


To further improve the performance you could try to derive your own list-view-item from the

public class ListViewItem

that luckily isn't sealed. Then instead of re-adding all items you could just refresh the items and the list-view could just update the values with ListView.Refresh(). To keep track of the list-view-items and processes you could use another dictionaray and if a new process is added or removed you add/remove just this one and not all of them.


Design

private Dictionary<string, Action> sortingActions;
private Dictionary<string, Action> orderingActions;
private bool isAscendingOrder = false;

I find these three fields very confusing because they sound so similar.

How about this. First rename this one

sortingActions -> selectColumn

With the new defintion you selects the column (value) without invoking sorting yet:

private Dictionary<string, Func<Process, IComparable>> selectColumn;
selectColumn = new Dictionary<string, Func<Process, IComparable>>
{
 ["Name"] = p => p.ProcessName,
 ["Status"] = p => p.Responding,
 ["Start Time"] = p => p.StartTime,
 ["Total Runtime"] = p => GetRuntimeOfProcess(p),
 ["Memory Usage"] = p => GetMemoryUsageInMB(p),
 ["Active Time"] = p => GetActiveTimeOfProcess(p)
};

Then rename the other dictionary to

orderingActions -> orderBy

where the key is no longer a string but an enum:

enum OrderBy
{
 Ascending,
 Descending
}

so the new dictionary has now stronger keys and its items trigger the sort function by using the first dictionary to get the delegate for getting the column (value)

orderBy = new Dictionary<OrderBy, Action>
{
 [OrderBy.Ascending] = () =>
 {
 SortProcesses(selectColumn[orderByColumn]);
 currentOrderBy = OrderBy.Ascending;
 },
 [..]
}

where orderByColumn is the name of the column to be ordered by that you set somewhere.

The isAscendingOrder now becomes currentOrderBy

private OrderBy currentOrderBy = OrderBy.Descending;

(disclaimer: notepad coding, may not be 100% correct yet)

t3chb0t
  • 44.6k
  • 9
  • 84
  • 190
default

AltStyle によって変換されたページ (->オリジナル) /