12

In Delphi XE7, I use this trick to automatically enable or disable a toolbar button ("Edit ListView Item") according to whether an item in the ListView is selected or not, to prevent the user to click on the button if there is no ListView Item selected:

  • Put a TActionList on a VCL Form.
  • In the ActionList create an action actTest.
  • Put a TButton on the form.
  • Assign the action actTest to the button.
  • Put a TListView on the form.
  • In the ListView create two items.
  • In the OnUpdate event of the actTest action write:

     procedure TForm1.actTestUpdate(Sender: TObject);
     begin
       actTest.Enabled := ListView1.SelCount > 0;
       CodeSite.Send('actTestUpdate'); // gets fired very often!
     end;
    

Now you can see that the button becomes enabled or disabled according to whether an item in the ListView is selected or not, independently from whether you select/deselect items with the mouse or with the keyboard or programmatically.

However, in the CodeSite Live Viewer I can see that the actTestUpdate event is fired continuously and very often, so the statement actTest.Enabled := ListView1.SelCount > 0; gets executed VERY OFTEN.

So my question is: Does this degrade the performance? If yes, is there another trick to achieve the above purpose?

Sir Rufo
  • 18,395
  • 2
  • 39
  • 73
user1580348
  • 5,721
  • 4
  • 43
  • 105
  • The `TAction.OnUpdate` intention is to do what you are doing, see http://docwiki.embarcadero.com/CodeExamples/XE7/en/TActionOnUpdate_(Delphi) – Sir Rufo Mar 01 '15 at 11:44
  • You are doing it as was intended. But, it is an unecessary wasting of resources. What's worse, there might be situations where you need to e.g. disable a certain action in the middle of a method execution and using the `OnUpdate` way may lead to doing it *unsafely late*. I'm trying to completely avoid `OnUpdate` personally. – TLama Mar 01 '15 at 12:09
  • 1
    @TLama Which alternative would you suggest? – user1580348 Mar 01 '15 at 12:38
  • I would update the action state right when something happens. In this case when the list view's underlying model selection changes. – TLama Mar 01 '15 at 12:51
  • @TLama How would you do that? I've tried them all. Remember, the goal is to update the button independently from whether you select/deselect items with the mouse or with the keyboard. – user1580348 Mar 01 '15 at 13:00
  • The crux is that TListView.OnSelectItem is triggered several times on selection change, and the various click and keyboard events each cover only either the mouse or the keyboard. So TListView lacks a proper event type for this purpose. – user1580348 Mar 01 '15 at 13:06
  • The `OnSelectItem` event should be fired only when the selection state of an item changes and it is so the ideal trigger for this update (whether you update a model, or the action itself when having no view model concept). That's better than wasteful sending of `LVM_GETSELECTEDCOUNT` messages to get the selected item count whenever the application goes idle (and nothing in the selection actually changes). – TLama Mar 01 '15 at 13:33
  • @TLama _...may lead to doing it unsafely late_ in which case you can update manually anyway. – NGLN Mar 01 '15 at 13:37
  • OnSelectItem fires twice when a selection is changing. Once with "selected" false, disable your button there. And once with "selected" true, enable your button then. Doesn't look like excessive processing.. – Sertac Akyuz Mar 01 '15 at 13:45
  • As I read it your OnUpdate is being fired repeatedly even when there is no user interaction. Is that so? If so, please make a repro. – David Heffernan Mar 01 '15 at 14:07
  • @David, [`OnUpdate`](http://docwiki.embarcadero.com/Libraries/XE7/en/System.Classes.TBasicAction.OnUpdate), *occurs when the application is idle...*, that's where the wasting origins. – TLama Mar 01 '15 at 14:11
  • @TLama It occurs when the app **becomes** idle. Not when it **is** idle. That means, when the message queue has just been emptied. So if no input messages, or timers, or paint messages are pulled from the queue it won't fire. Now, if user has a timer running, OnUpdate will happen after every timer event. – David Heffernan Mar 01 '15 at 14:16
  • @David I'm pretty sure OP means by _continuously and very often_ that he didn't notice that without mouse movement. – NGLN Mar 01 '15 at 14:26
  • @NGLN I'd like to be sure. If user is sitting there providing no input and OnIdle is firing repeatedly, it would be nice to know why. I've see OnIdle lead to significant CPU utilization when an app is idle. – David Heffernan Mar 01 '15 at 14:29
  • @David, which happens still unecessarily often (and *randomly*) for such purpose. I'm happy to update my action states just when they need, not when the application becomes idle, sun goes down, or moon enters a certain phase :) – TLama Mar 01 '15 at 14:33
  • @DavidHeffernan Yes, I am sitting here at my desk, no mouse moving, no keyboard typing, having no timer running and watching the messages running down like a waterfall in CodeSite Live Viewer. It's very relaxing, better than an aquarium... – user1580348 Mar 01 '15 at 18:25
  • @user1580348: Then there is most likely a timer running in your code, or maybe inside one of your UI controls. You can use the `TApplication.OnMessage` event to verify that. – Remy Lebeau Mar 01 '15 at 18:39
  • Yes, timer is the likely explanation. OnIdle is quite crude. – David Heffernan Mar 01 '15 at 18:47
  • I was hit by the same problem, if you have many actions you will find high cpu usage, even when you do nothing, you can set `Application.ActionUpDateDelay` to higher value so that the event fires less often. – whosrdaddy Mar 02 '15 at 07:27

3 Answers3

9

If you have (or plan to have) many actions you might want to set Application.ActionUpdateDelay to e.g. 50 milliseconds. This can improve the performance noticeable.

Also, again if you have many actions, I would suggest you try to use TForm.UpdateActions instead of defining TAction.OnUpdate for each action. It will make the code more readable.

pyxidata
  • 131
  • 6
5

Generally

Yes, an OnUpdate event handler takes time, just as any other routine does. Multiple handlers take a multiple of that time. The gross of all that code will evaluate conditions resulting in just nothing to do. In that sense, you could conclude that this update mechanism degrades performance. Especially considering these update events occur quite often:

Occurs when the application is idle or when the action list updates.

That could be a reason to spare its use. But you should realize that the evaluation of a single expression mostly does not take that much time. Also, realize that regardless of action updates, your application performs (much more heavy) calculations and operations on every single mouse move.

When you keep the code duration in action update events to a minimal, e.g. no password checking via a database connection, then performance will appear just normal. If you háve lengthy operations linked to updating actions, then fall back on manual updates in those specific situations.

Note that performance can be slightly gained by not using the individual OnUpdate events of the Actions, but the OnUpdate event of the ActionList instead which has a Handled parameter to cancel further processing, with the additional benefit of centralization and categorization.

Specifically

Your ListView1.SelCount sends a WinAPI message to the control to retrieve the selection count. That is a tiny operation and I would not bother its time needed.

An alternative is to update the action in the ListView's OnSelectItem event. That event will catch all selection changes due to mouse and keyboard interaction, as well as setting the individual items' Selected property:

procedure TForm1.ListView1SelectItem(Sender: TObject; Item: TListItem;
  Selected: Boolean);
begin
  actTest.Enabled := ListView1.SelCount > 0;
end;

However, the ListView nor the VCL do not provide anything to signal only between SelCount = 0 and SelCount > 0, so you will evaluate that line of code more then strictly necessary anyway.

Assuming MultiSelect is true, you could also count the selection changes yourself to eliminate the need for calling SelCount:

  private
    FListViewSelected: Longbool;

procedure TForm1.ListView1SelectItem(Sender: TObject; Item: TListItem;
  Selected: Boolean);
begin
  if Selected then
    Inc(FListViewSelected)
  else
    Dec(FListViewSelected);
  actTest.Enabled := FListViewSelected;
end;

Or test for the selected item being nil:

procedure TForm1.ListView1SelectItem(Sender: TObject; Item: TListItem;
  Selected: Boolean);
begin
  actTest.Enabled := ListView1.Selected <> nil;
end;

But then again, there really is no reason left for not using the OnUpdate event:

procedure TForm1.ActionList1Update(Action: TBasicAction; var Handled: Boolean);
begin
  actTest.Enabled := ListView1.Selected <> nil;
  Handled := True;
end;
Community
  • 1
  • 1
NGLN
  • 43,011
  • 8
  • 105
  • 200
0

The Action update events are (mostly) executed inside Application.Idle. As long as you don't do time critical things inside the event handlers there should be no noticeable performance degradation.

Uwe Raabe
  • 45,288
  • 3
  • 82
  • 130
  • 1
    It's not about time criticality (the thing itself depends on time spend), but about time taking/duration (the thing itself having consequence for things to come). – NGLN Mar 01 '15 at 12:38