A few clarifications on the new version

Please check the FAQ (https://www.xyplorer.com/faq.php) before posting a question...
RalphM
Posts: 1932
Joined: 27 Jan 2005 23:38
Location: Cairns, Australia

Re: A few clarifications on the new version

Post by RalphM »

Is this thread your container for all ideas XY?
It would be a lot easier for others & yourself to find old threads again if they focused on one subject only and have a descriptive subject line.

Time stamping can easily be achieved with CFA's or from the properties info panel.
Ralph :)
(OS: W11 22H2 Home x64 - XY: Current beta - Office 2019 32-bit - Display: 1920x1080 @ 125%)

Antieve
Posts: 72
Joined: 02 Apr 2016 20:52

Re: A few clarifications on the new version

Post by Antieve »

RalphM wrote: 20 May 2022 04:11Time stamping can easily be achieved with CFA's or from the properties info panel.
to have native commands
But if the script can permanently add these commands to the regular menu, then I agree.
Not sure what did you mean about "properties info panel".

RalphM
Posts: 1932
Joined: 27 Jan 2005 23:38
Location: Cairns, Australia

Re: A few clarifications on the new version

Post by RalphM »

Info panel / Properties Tab:
PicPick_662.jpg
PicPick_662.jpg (25.16 KiB) Viewed 2574 times
The file dates on the right side are editable if you click on them.

For native menu changes you need to convince Don to add something there.
Ralph :)
(OS: W11 22H2 Home x64 - XY: Current beta - Office 2019 32-bit - Display: 1920x1080 @ 125%)

Antieve
Posts: 72
Joined: 02 Apr 2016 20:52

Re: A few clarifications on the new version

Post by Antieve »

if you click on them
Good to know, thx!
If you do not need absolute precision, it is also a convenient way.

In fact, I suggested this improvement only because it is easy to implement, It is clear that this is not critical at all :) :beer:

Antieve
Posts: 72
Joined: 02 Apr 2016 20:52

Re: A few clarifications on the new version

Post by Antieve »

Hi
It's been almost half a year since we talked and I would love to upgrade to a new version of the XY, but I can't because
Antieve wrote: 13 May 2022 07:11 Something goes wrong with realization because:
  1. XYCtx window spamed with WM_TIMER messages all the time
  2. XYCtx have a memory leak
I can understand the reason why you did not make a fast implementation with data objects, but even with the current implementation I think it is unacceptable to leave such critical bugs...


We can certainly live with this internal mini DDoS attack

Image

But an endless memory leak is too much :kidding:

Image

was too lazy to push it to the limit, I think 2GB is enough for clarity.
I've written about it more than once before, but to my surprise it still hasn't been fixed... :cry:

admin
Site Admin
Posts: 60357
Joined: 22 May 2004 16:48
Location: Win8.1 @100%, Win10 @100%
Contact:

Re: A few clarifications on the new version

Post by admin »

I don't see that here. Which Windows version is that?

Antieve
Posts: 72
Joined: 02 Apr 2016 20:52

Re: A few clarifications on the new version

Post by Antieve »

admin wrote: 07 Nov 2022 08:49 I don't see that here. Which Windows version is that?
I have Windows 10 but this behavior should not depend on the OS version.

To see the first problem, simply run any window message monitor and select the XY64ctxmenu window with the "AutoHotkey" class. The screenshot from the previous post shows the output from spy++

Image

If you don't have one I have attached the archive to the post.

To reproduce memory leak quickly and clearly, you need to call the context menu for a large number of files. Each such call will increase the amount of memory used. Repeat until you get bored or run out of memory on your PC :mrgreen:
For convenience, use the cmd file from this post to quickly create a folder with many files.

You can open the TaskManager or another tool in the background (I use ProcessHacker 3.0.4953 because it has a lot more features and I don't like Process Explorer) to see the process memory in real time after each menu call.
Attachments
Spy++.7z
(300.06 KiB) Downloaded 59 times

admin
Site Admin
Posts: 60357
Joined: 22 May 2004 16:48
Location: Win8.1 @100%, Win10 @100%
Contact:

Re: A few clarifications on the new version

Post by admin »

1) The messages are harmless. They are used for auto-close. I could slow them down a bit.

2) The leak is there, but I don't see the hole. :|

Antieve
Posts: 72
Joined: 02 Apr 2016 20:52

Re: A few clarifications on the new version

Post by Antieve »

It's been a long time since I looked at the source code and I must admit that XY64ctxmenu code is much cleaner now.

But with window messages I'm not quite clear, what is the algorithm of communication between processes?
Why do you need to "ping" a window?

admin
Site Admin
Posts: 60357
Joined: 22 May 2004 16:48
Location: Win8.1 @100%, Win10 @100%
Contact:

Re: A few clarifications on the new version

Post by admin »

In the theoretical case that XY goes down (which of course never happens in reality), the XY64ctxmenu process closes itself.

Antieve
Posts: 72
Joined: 02 Apr 2016 20:52

Re: A few clarifications on the new version

Post by Antieve »

admin wrote: 10 Nov 2022 07:02 In the theoretical case that XY goes down (which of course never happens in reality), the XY64ctxmenu process closes itself.
There is a proper way to do this.
Another reason to start XY64ctxmenu at once together with XYplorer ;)

admin
Site Admin
Posts: 60357
Joined: 22 May 2004 16:48
Location: Win8.1 @100%, Win10 @100%
Contact:

Re: A few clarifications on the new version

Post by admin »

Good to know, thanks. When I find the time...

Antieve
Posts: 72
Joined: 02 Apr 2016 20:52

Re: A few clarifications on the new version

Post by Antieve »

Had some time, wrote a note :)

CURRENT IMPLEMENTATION

As for the memory leak, the reason is this cycle:

Code: Select all

Loop
    {
      SHParseDisplayName(pidl)         ; <-- MAIN REASON: each call creates a new instance of ITEMIDLIST, and only the last remaining instance in the 'pidl' variable is freed
      SHBindToParent(pIShellFolder)    ; I have not checked if on each loop returns the same ppv pointer (logically should be the same), but if not, you must release every one of them
    } 
CoTaskMemFree(pidl)
ObjRelease(pIShellFolder)
In fact, SHBindToParent needs to be called just once to get a pointer to the IShellFolder interface of the parent object (In our case, the parent folder is the same for all passed objects).

The "pure" call chain looks like this:

SHGetDesktopFolder(Desktop.IShellFolder)
  Desktop\BindToObject(ParentIShellFolder.IShellFolder)    ;(you prefer SHBindToParent)
    ParentIShellFolder\GetUIObjectOf(IContextMenu.IContextMenu)
      IContextMenu\QueryInterface(IContextMenuX.IContextMenu[2\3])
        IContextMenuX\QueryContextMenu()


And SHParseDisplayName just converter [path > ITEMIDLIST] it returns a pointer to the ITEMIDLIST (aka PIDL), which is then stored in an array for GetUIObjectOf.



POSSIBLE IMPROVEMENTS
  1. Create the XY64ctxmenu.exe process immediately after starting XYplorer
    • This will avoid a delay when calling the context menu for the first time
    • Startup argument - XYplorer's window handle (although it will be passed in the wParam of WM_COPYDATA, but why not)
    • Use a Job Objects to auto-kill XY64ctxmenu
      NOTE: Set JOB_OBJECT_LIMIT_BREAKAWAY_OK (to child processes of XY64ctxmenu are not associated with the job)
      otherwise all programs started with XYplorer will also be terminated
      Using Job Objects will avoid unnecessary code and spam messages
  2. For code speed, it is critical to avoid all strings operations wherever possible:
I have never coded in VB6, but surely the list of selected files at some point is in the one of the shell object format i.e., as an ITEMIDLIST
SHParseDisplayName is called at least once for the selected item
    [0x0]            : shell32!SHParseDisplayName
    [0x1]            : shell32!SHGetFileInfoW + 0xd5
    [0x2]            : shell32!SHGetFileInfoWStub + 0x17
    [0x3]            : XYplorer + 0x3e8de0
    [0x4]            : XYplorer + 0x2a41de
    [0x5]            : XYplorer + 0x29fa02
    [0x6]            : XYplorer + 0x1e2539
    [0x7]            : XYplorer + 0x1f0a53

so we already have a ready-made ITEMIDLIST somewhere...

Actually it is rather sad that now we first extract paths from ITEMIDLIST, combine them into a string, send it, then split it again and convert it into ITEMIDLIST one more time and then create an array for GetUIObjectOf.
A lot of unnecessary work, if you ask me...

By far the best ways would be to pass an CFSTR_SHELLIDLIST or DataObject or ITEMIDLIST (in a message "body" or as a pointer to shared memory) instead of one big string.
In the latter case you can even go further and read it directly from the XYplorer process memory instead of data transferring (others cases use global memory).

In the worst case if for some reason this is not possible, then send message with following format:
---------------------------------------------------------------------------------------------------------------------------------------
   *Reason:BYTE    Byte containing the call reason code (menu|open|openwith)
   *Flags:BYTE       Byte containing extra options (EXTENDEDVERBS)  
                            although it can be eliminated by adding variations of the first position (aka menu|menuex)
   *Mouse:POINT    Qword with mouse coordinates
   *in:USHORT       The number of items that are being transferred
   *DATA:DtArray   Array of paths
---------------------------------------------------------------------------------------------------------------------------------------


DtArray 
  {
    USHORT cb         The size of the path string, in bytes, includes the 2 bytes of the size itself, 0 if item is empty (last one)
    BYTE   path[1]    A variable-length path
   }


Example with 2 files (Unicode)

Code: Select all

  cb   path   cb   path   cb
 ┌──┬────────┬──┬────────┬──┬┐
 │18│C:\1.txt│18│C:\2.txt│00││
 └──┴────────┴──┴────────┴──┴┘
And instead of slow string operations you'll get fast pointer math
The first pointer *Ptr is message\DtArray offset, second one *Ptr + *Ptr\cb etc until cb = 0
Working with strings is completely eliminated, just pass pointers to SHParseDisplayName in the loop.

I'm not entirely sure why you need to pass the mouse coordinates, but if you need them for something, use the appropriate POINT format (Parsing a string with RegEx for this is some kind of insane :) )

And one more thing.
You create a XYplorer's message loop with PeekMessage but in my opinion GetMessage is much preferable since at idle it does not waste CPU cycles.

:beer:

Post Reply