Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: batch tmux show-options #240

Merged
merged 6 commits into from
Aug 5, 2024
Merged

Conversation

vdbe
Copy link
Contributor

@vdbe vdbe commented Jun 26, 2024

Batching tmux show-options gives a significant start uptime decrease of up to 3 under linux (tested with bath bash-5.2.26 and 3.2.57).

On the bash version used by macos this however seems to have a negative impact. This was test in docker with as image bash:3.2.57-alpine3.19. The negative impact maybe from docker/alpine since in bash:5.1-alpine3.19 it also had a negative impact but less.
Can anyone on macos see what the real effect is?

I think the the complexity increase is worth it if the "macos"/old bash/alpine hit is because of alpine, but would love to get more feedback on this.
Effect was docker/alpine build bash-3.2.57 and it works great.

Tested on 10 user configs (from issues) with and without batching and batching was always an improvement.

With defaults in the array is most of the time slower but did not spend any time in looking for improvements since the with defaults is a bit more user friendly.

  • Example config 1
    Screenshot from 2024-07-14 16-32-33

  • Example config 2
    Screenshot from 2024-07-14 16-35-22

  • Example config 3
    Screenshot from 2024-07-14 16-38-49

BEGIN_COMMIT_OVERRIDE
perf: batch tmux show-options
END_COMMIT_OVERRIDE

@vdbe

This comment was marked as outdated.

@vdbe
Copy link
Contributor Author

vdbe commented Jul 9, 2024

I have been using this PR for 2 weeks now and everything seems to work.

Would love to have the performance increases/decreases from other people/platforms so we can make a better decision on including this or not.

The bash script below will 'benchmark' this PR and the main branch using hyperfine if installed (more accurate/useful data) and time if not.
It needs to be ran when inside a tmux pane/window.
It would be really helpful if you could run this and comment the output/result (and if it's linux/macos/wsl/...).

#!/usr/bin/env bash

# Clone both repositories/branches
git clone --depth=1  https://github.com/catppuccin/tmux.git catppuccin-tmux
git clone --depth=1 --branch perf/batch-tmux-show https://github.com/vdbe/catppuccin-tmux.git catppuccin-tmux-batch

# Print bash and tmux version
echo "bash: ${BASH_VERSION}"
echo "tmux: $(tmux -V)"

# Benchmark
if command -v hyperfine 
then
  # hyperfine is installed
  hyperfine --warmup 3 -N ./catppuccin-tmux/catppuccin.tmux ./catppuccin-tmux-batch/catppuccin.tmux
else
  # hyperfine is not installed just using sleep
  echo ./catppuccin-tmux/catppuccin.tmux
  time ./catppuccin-tmux/catppuccin.tmux
  echo ./catppuccin-tmux-batch/catppuccin.tmux
  time ./catppuccin-tmux-batch/catppuccin.tmux
fi

benchmark.bash.txt

Example outputs
bash: 5.2.26(1)-release
tmux: tmux 3.4
./catppuccin-tmux/catppuccin.tmux

real	0m0.450s
user	0m0.231s
sys	0m0.183s
./catppuccin-tmux-batch/catppuccin.tmux

real	0m0.263s
user	0m0.144s
sys	0m0.110s
bash: 5.2.26(1)-release
tmux: tmux 3.4
/nix/store/9hv3swqri0zg5iqaw0v4mxsycxc5vgbb-hyperfine-1.18.0/bin/hyperfine
Benchmark 1: ./catppuccin-tmux/catppuccin.tmux
  Time (mean ± σ):     399.6 ms ±   3.0 ms    [User: 221.4 ms, System: 157.6 ms]
  Range (min … max):   396.6 ms … 406.4 ms    10 runs

Benchmark 2: ./catppuccin-tmux-batch/catppuccin.tmux
  Time (mean ± σ):     244.1 ms ±   2.3 ms    [User: 131.4 ms, System: 106.2 ms]
  Range (min … max):   241.7 ms … 248.5 ms    12 runs

Summary
  ./catppuccin-tmux-batch/catppuccin.tmux ran
    1.64 ± 0.02 times faster than ./catppuccin-tmux/catppuccin.tmux

utils/tmux_utils.sh Outdated Show resolved Hide resolved
@uncenter
Copy link
Member

uncenter commented Jul 9, 2024

On macOS:

bash: 5.2.26(1)-release
tmux: tmux 3.4
/etc/profiles/per-user/uncenter/bin/hyperfine
Benchmark 1: ./catppuccin-tmux/catppuccin.tmux
  Time (mean ± σ):     230.6 ms ±  43.4 ms    [User: 87.4 ms, System: 90.1 ms]
  Range (min … max):   209.6 ms … 343.2 ms    10 runs

  Warning: The first benchmarking run for this command was significantly slower than the rest (343.2 ms). This could be caused by (filesystem) caches that were not filled until after the first run. You should consider using the '--warmup' option to fill those caches before the actual benchmark. Alternatively, use the '--prepare' option to clear the caches before each timing run.

Benchmark 2: ./catppuccin-tmux-batch/catppuccin.tmux
  Time (mean ± σ):      99.8 ms ±  38.7 ms    [User: 38.6 ms, System: 41.8 ms]
  Range (min … max):    88.4 ms … 228.7 ms    13 runs

  Warning: The first benchmarking run for this command was significantly slower than the rest (228.7 ms). This could be caused by (filesystem) caches that were not filled until after the first run. You should consider using the '--warmup' option to fill those caches before the actual benchmark. Alternatively, use the '--prepare' option to clear the caches before each timing run.

Summary
  ./catppuccin-tmux-batch/catppuccin.tmux ran
    2.31 ± 1.00 times faster than ./catppuccin-tmux/catppuccin.tmux

@vdbe vdbe marked this pull request as ready for review August 5, 2024 09:11
@vdbe vdbe merged commit 3c6f6f2 into catppuccin:main Aug 5, 2024
3 checks passed
vdbe added a commit to vdbe/catppuccin-tmux that referenced this pull request Aug 6, 2024
Changes the line separator used by tmux_batch from ':' to the ascii unit
separator

FIXES: catppuccin#240
vdbe added a commit that referenced this pull request Aug 6, 2024
vdbe added a commit that referenced this pull request Aug 6, 2024
vdbe added a commit to vdbe/catppuccin-tmux that referenced this pull request Aug 6, 2024
Changes the line separator used by tmux_batch from ':' to the ascii unit
separator

FIXES: catppuccin#240
vdbe added a commit to vdbe/catppuccin-tmux that referenced this pull request Aug 10, 2024
Changes the line separator used by tmux_batch from ':' to the ascii unit
separator

FIXES: catppuccin#240
vdbe added a commit that referenced this pull request Aug 12, 2024
* feat: batch tmux show options

* dont check for duplicates

* fix: status modules

* batch and cache build_window_icon

* perf: batch new options

* chore: tmux_batch_setup_module -> tmux_batch_setup_status_module

* fixup! perf: batch tmux show-options (#240)

Changes the line separator used by tmux_batch from ':' to the ascii unit
separator

FIXES: #240

* fix: string escaping of `..._tmux_batch_...`

`tmux show -v <option>` returns/prints the escaped value while the value string
of `tmux show <option>` is not escaped.

FIXES: #281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants