[BUG] MatchedItem should be `Weak<dyn SkimItem>` to prevent cycles

[kimono-koans]

Describe the bug
Chained invocations of skim retain lots of memory. I believe there is cycle within skim which prevents Arc<dyn SkimItem> from being dropped.

pub struct MatchedItem {
    /// The underlying skim item
    pub item: Arc<dyn SkimItem>,
    /// The rank/score of this match
    pub rank: Rank,
    /// Range of characters that matched the pattern
    pub matched_range: Option<Box<MatchRange>>,
}

To Reproduce
Steps to reproduce the behavior:

1. Invoke skim as a library.
2. Allow skim to build up a substantial number of SkimItems (hundreds of thousands).
3. Search the SkimItems a few times.
4. Exit skim invocation, but keep a main process running. skim retains hundreds megabytes of memory (hundreds of megabytes, even a gigabyte of non-purgeable memory).

Expected behavior
For skim to deallocate as much memory as possible.

Environment (please complete the following information):

• OS (uname -a): MacOS 26.3 and Ubuntu 24.04.4
• skim version (sk --version): 3.2.0
• Shell and version: zsh 5.9-6ubuntu2
• Variables (env | grep '^SKIM'): None

[LoricAndre]

I'll work on this, and make it so that only the pool actually "owns" the items, and other parts of the code (MatchedItem, item list Selection...) only refer to those. This way, dropping the item pool will be sure to drop all the items.

[LoricAndre]

I spent quite some time diagniosing this, and I might have found the culprit : could you try adding a unsafe { nix::libc::malloc_trim(0); }; block after the run_with call and tell me if that fixes it ?

[LoricAndre]

As far as I can tell, no memory leaks from our current Arc approach, though it does look a bit unoptimized. It seems to originate to Rust's allocator keeping the memory pages for further reallocations, and getting confused at some point later. Calling the malloc_trim(0) freed everything on my side between runs.

[kimono-koans]

I spent quite some time diagniosing this, and I might have found the culprit : could you try adding a unsafe { nix::libc::malloc_trim(0); }; block after the run_with call and tell me if that fixes it ?

Still seem to be holding onto lots of memory.

PID    COMMAND      %CPU TIME     #TH    #WQ   #PORT MEM    PURG   CMPRS  PGRP  PPID  STATE    BOOSTS             %CPU_ME %CPU_OTHRS UID  FAULTS
43078  httm         0.1  01:04.88 35     0     50    1815M  0B     592K   43078 42969 sleeping *0[1]              0.00000 0.00000    501  352701

As far as I can tell, no memory leaks from our current Arc approach, though it does look a bit unoptimized. It seems to originate to Rust's allocator keeping the memory pages for further reallocations, and getting confused at some point later. Calling the malloc_trim(0) freed everything on my side between runs.

How are you testing chained runs?

[LoricAndre]

I am using the multiple_runs example to check the resources across runs

[LoricAndre]

Can you try the latest master then ? I added something else to make sure everything is dropped before we return the output

[kimono-koans]

Can you try the latest master then ? I added something else to make sure everything is dropped before we return the output

Switched to Linux. On master using malloc_trim:

    PID USER      PR  NI    VIRT    RES    SHR S  %CPU  %MEM     TIME+ COMMAND
...
 236258 rswinfo+  20   0 1031524  92724   4780 S   0.3   0.6   0:34.33 httm

[LoricAndre]

So it does have something to do with the allocator, it seems :/
I don't know enough about MacOS's system allocator to be of much help I'm afraid

[kimono-koans]

So it does have something to do with the allocator, it seems :/ I don't know enough about MacOS's system allocator to be of much help I'm afraid

Some obviously does have to with the allocator, but isn't 92724/92M a bunch to still hold onto?

[LoricAndre]

I don't think Skim is holding onto that memory. Before Skim::output() is called (at the end of the static Skim::run_with()), the Skim instance itself is dropped, and it owns everything skim uses. Rust being rust, as there is no unsafe code on the hotpath, by that point everything has been dropped and no memory should remain in use by skim itself. 92M could be the items you sent still being stored somewhere from before you sent them, or another source. I don't see anywhere where skim itself could hog memory once everything is dropped: even with Arc<dyn SkimItem> there are no cycles anywhere and once everything is dropped the Arc's contents will be deallocated.

[LoricAndre]

Can you try again with the latest version if you have some time ? I switched the allocator to mimalloc for performance reasons, but it might actually help with this too

[kimono-koans]

Can you try again with the latest version if you have some time ? I switched the allocator to mimalloc for performance reasons, but it might actually help with this too

Not seeing any better memory performance after a few searches.

I am seeing some other weird behavior. Yikes. After I delete my query, or the query returns to empty, the items are not sorted in their original order?

[LoricAndre]

That's a weird one, the matcher should run with the empty query in both cases, so the tiebreak should be applied the same, but if you could open a new issue with a reproduction it could be useful