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

Loading assets from the cache is significantly slower than from the binary #1753

Closed
Enselic opened this issue Jul 26, 2021 · 0 comments · Fixed by #1754 or sthagen/sharkdp-bat#31
Closed
Labels
bug Something isn't working performance

Comments

@Enselic
Copy link
Collaborator

Enselic commented Jul 26, 2021

Step-by-step:

  1. Measure the time it takes bat to highlight examples/simple.rs without cached assets, i.e. when the assets integrated into the binary are used:
% rm -rf ~/.cache/bat
% hyperfine --export-markdown /dev/tty 'bat --color always examples/simple.rs'
Command Mean [ms] Min [ms] Max [ms] Relative
bat --color always examples/simple.rs 107.4 ± 2.1 104.1 112.5 1.00
  1. Create cached assets that are identical to the integrated assets:
% mkdir -p ~/.cache/bat
% echo "---\nbat_version: 0.18.2" > ~/.cache/bat/metadata.yaml
% cp assets/syntaxes.bin ~/.cache/bat/syntaxes.bin
% cp assets/themes.bin ~/.cache/bat/themes.bin
  1. Measure again
% hyperfine --export-markdown /dev/tty 'bat --color always examples/simple.rs'
Command Mean [ms] Min [ms] Max [ms] Relative
bat --color always examples/simple.rs 115.0 ± 2.1 111.9 120.1 1.00

Expected result

There is no significant difference.

Actual result

There is a significant difference.

I will soon create a PR with a proposed fix.

@Enselic Enselic added bug Something isn't working performance labels Jul 26, 2021
Enselic added a commit to Enselic/bat that referenced this issue Jul 26, 2021
Using BufReader makes sense for large files, but assets are never large enough
to require buffering. It is significantly faster to load the file contents in
one go, so let's do that instead.

Closes sharkdp#1753
Enselic added a commit to Enselic/bat that referenced this issue Jul 26, 2021
Using BufReader makes sense for large files, but assets are never large enough
to require buffering. It is significantly faster to load the file contents in
one go, so let's do that instead.

Closes sharkdp#1753
Enselic added a commit that referenced this issue Jul 27, 2021
Using BufReader makes sense for large files, but assets are never large enough
to require buffering. It is significantly faster to load the file contents in
one go, so let's do that instead.

Closes #1753
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working performance
Projects
None yet
1 participant