Recent

Author Topic: [SOLVED] 2.2 win x64: ugly issue with virtual listview "Selected" performance  (Read 11642 times)

d7_2_laz

  • Sr. Member
  • ****
  • Posts: 267
Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
« Reply #30 on: January 23, 2022, 04:01:28 pm »
Fully understandable GetMen ....  and many thanks for all your replies here as well as in the past, which i highly appreciate!

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4097
  • I like bugs.
Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
« Reply #31 on: January 27, 2022, 07:28:42 pm »
Please test with latest trunk (main).
See my note in issue
 https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39324
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

d7_2_laz

  • Sr. Member
  • ****
  • Posts: 267
Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
« Reply #32 on: January 27, 2022, 10:14:12 pm »
Juha, great thanks for this attention!

As i hadn't time today to reestablish a recent trunk test,  i tried it - after inspecting the patch and, so, having no doubt that it would work :-)  - to test with an accordingly adapted customlistview.inc.
In short i retried, after applying this patch,  all my test cases related.  Result just as expected: that works well and solves most of the performance problems as discussed here. It helps!

One very little thing needs to be mentioned though (not occuring with a pure 2.2 customlistview.inc).  Ownerdata + Multiselect + patch:   "Shift"  and VK_UP will return nil as result value for "Selected".   Test project and workaround proposal (a simple 2-liner) do exist. I hadn't that mentioned here yet for not to flood even more stuff or confusion.

As the patch is highly appreciated from my side, i'd be happy we could keep this little thing completely apart from here (ok?), as the patch does solve the urgest pain.

(yes there might be some performance issues lieft, ie. when looping around all items and asking if Items.Selected, but this is nothing new or directly related to this specific topic)  The patch does already help a lot)

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4097
  • I like bugs.
Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
« Reply #33 on: January 28, 2022, 11:48:29 am »
One very little thing needs to be mentioned though (not occuring with a pure 2.2 customlistview.inc).  Ownerdata + Multiselect + patch:   "Shift"  and VK_UP will return nil as result value for "Selected".   Test project and workaround proposal (a simple 2-liner) do exist. I hadn't that mentioned here yet for not to flood even more stuff or confusion.
Is the attached patch OK? I could not reproduce the problem on Linux.
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

d7_2_laz

  • Sr. Member
  • ****
  • Posts: 267
Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
« Reply #34 on: January 28, 2022, 12:55:47 pm »
Yes! (if i interprete the patch file right):  that matches exactly the workaround i would have proposed:
'
Code: Pascal  [Select][+][-]
  1.  
  2. // (TCustomListView.CNNotify, around line 359 in the recent trunk file:
  3.                if (nm^.iItem < 0) or (nm^.iItem = FSelectedIdx) then
  4.                  // Replace this line ...
  5.                  //InvalidateSelected;
  6.                  // ... by these lines:
  7.                    if not MultiSelect then
  8.                       InvalidateSelected;
  9.  

. Obviously, without this,  InvalidateSelected would be executed too general here
- Ccouldn't see yet any negative effect when suppressing it for MultiSelect

Unless there are more basic considerations i think as of today it's the best me could do.

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4097
  • I like bugs.
Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
« Reply #35 on: January 28, 2022, 04:33:35 pm »
Applied. The relevant changes will be merged to 2.2.2.

Yes! (if i interprete the patch file right):  that matches exactly the workaround i would have proposed:
A patch does not need interpreting. It represents the change exactly. The tool chain supports that exact information, meaning patches can be generated and applied easily.

You really must learn to use the development branch "main" and to work with patches.
Everybody involved in the development (as you are now) really has to do it. It is super-easy. Did you even try?
Now I had to dig the suggested changes from long forum threads etc.

Merge requests in GitLab are an alternative to patches. I personally don't like them. Typically a merge request is associated with a bug report, thus splitting the relevant information into 2 places. To test a merge request I must download the changes as a patch and apply it. A patch uploaded into the original bug report would make more sense.
Trivial changes can be merged without testing by clicking a "Merge" button. Then a merge request is useful but those cases are rare.
Thus, a good old patch is still recommended.

I understood the iteration of selected ListView items could be optimized further. If you do it, please open a new report and attach a patch.
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

d7_2_laz

  • Sr. Member
  • ****
  • Posts: 267
I'd have to accept the critics. It's because i'm less familiar with the repository procedures tools and more comfortable to, when encountering a problem, find out the causing reason, make it reproducable and to search for possible ways out with having as best no side effects. Oftenly enough more overall insights will exist, ie. regarding cross-platform related impacts or conceptual reasons where to patch best.

With the patch the performance is back again, and happy enough about that, i don't really search for further optimizations, at least not now.

As the exchange so far of course was focused on the most important thing, i'll take the other initially mentioned (39325, only one item in the list: ...etc.) out here and adapt the topic's title accordingly.

Great thanks from my side to all who took part!

 

TinyPortal © 2005-2018