omerxx / tmux-sessionx

A Tmux session manager, with preview, fuzzy finding, and MORE
GNU General Public License v3.0
554 stars 51 forks source link

Slow startup time with many custom_paths #99

Closed IdoKendo closed 1 month ago

IdoKendo commented 1 month ago

Hey 👋 I've noticed that in cases when there are many custom_paths, this piece of code seems to be a bottleneck in startup time:

        for i in ${paths//,/$IFS}; do
            if grep -q $(basename $i) <<< $sessions; then
                continue
            fi
            echo "$i"
        done

In comparison, if I just use echo $paths | tr ' ' '\n' it runs in the blink of an eye. The downside is that it will show existing sessions' paths still, e.g. if I have a custom path ~/projects/tmux-sessionx and an existing session tmux-sessionx they will both appear.

Not sure what is the best solution here, maybe there's another option to improve the performance of the above loop that I'm not thinking of.

ProgmRuanSilva commented 1 month ago

Hey @IdoKendo, thanks for opening this issue.

Explaining this code:

  1. For each path, extracts the last component (filename) using basename.
  2. Checks if that filename is present in a list of sessions ($sessions) using grep.
  3. If it finds a match, skips processing further lines for this iteration.
  4. If no match is found, prints the original path.

This is a possible slowdown when there are many paths.

Alternatives that I would consider:

  1. Make a preprocessing of the list.
  2. Change to tr
IdoKendo commented 1 month ago

Hey @ProgmRuanSilva 👋

I was playing around a bit with it and I think that in order to keep the same logic, it's possible to use xargs to run this loop in parallel using --max-procs flag, I can open a PR if this solution makes sense. WDYT?

ProgmRuanSilva commented 1 month ago

Hey @ProgmRuanSilva 👋

I was playing around a bit with it and I think that in order to keep the same logic, it's possible to use xargs to run this loop in parallel using --max-procs flag, I can open a PR if this solution makes sense. WDYT?

We can make it in two ways:

  1. Background using:
    &>/dev/null
  2. Running in parallel with xargs. In my opinion, we don't need the sorts of precautions of the GNU Parallel

Important: The fzf is not asynchronous by default we need specify it using --sync like in this issue

If you can open the PR with these changes, I would appreciate it.

IdoKendo commented 1 month ago

Hey @ProgmRuanSilva I opened a PR. Would appreciate if you could review 🙏

ProgmRuanSilva commented 1 month ago

Hey @ProgmRuanSilva I opened a PR. Would appreciate if you could review 🙏

I will see tomorrow