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

refactor: update impatient #2477

Merged
merged 5 commits into from Apr 20, 2022
Merged

Conversation

kylo252
Copy link
Collaborator

@kylo252 kylo252 commented Apr 15, 2022

Description

  • update impatient: merge the the latest upstream version. it still needs to be loaded before any plugins, so it will have to be managed manually either way
  • feat: use an independent cache directory: overridden vim.fn.stdpath to use LUNARVIM_x_DIR instead, since a lot of plugins call this function interally

resolves #1256
fixes #2490

How to test

  • added new tests for vim.fn.stdpath:
    a.it("should be able to use lunarvim directories using vim.fn", function()
    assert.equal(lvim_runtime_path, vim.fn.stdpath "data")
    assert.equal(lvim_config_path, vim.fn.stdpath "config")
    assert.equal(lvim_cache_path, vim.fn.stdpath "cache")
    end)
    a.it("should be to retrieve default neovim directories", function()
    local xdg_config = os.getenv "XDG_CONFIG_HOME"
    assert.equal(join_paths(xdg_config, "nvim"), vim.call("stdpath", "config"))
    end)
  • check refactor: update impatient #2477 (comment)

Notes

For future references:

@kylo252 kylo252 marked this pull request as ready for review April 17, 2022 09:16
@abzcoding
Copy link
Member

tested with a fuck ton of plugins

  1. without this PR -> 240 to 260 ms
  2. with this PR(uncommented) -> 270 to 300 ms
  3. with this PR(setup commented) -> 320 to 600 ms

@kylo252
Copy link
Collaborator Author

kylo252 commented Apr 18, 2022

  1. without this PR -> 240 to 260 ms
  2. with this PR(uncommented) -> 270 to 300 ms

Are the cache files growing in size with this PR? I noticed that it might be more gradual with this new upstream implemention.

@abzcoding
Copy link
Member

Are the cache files growing in size with this PR? I noticed that it might be more gradual with this new upstream implemention.

I'll try it a bit more and report back

@abzcoding
Copy link
Member

abzcoding commented Apr 18, 2022

wtpr -> without this PR
pruc -> with this pr
prc -> with this pr and setup commented

State Cache Size Startup Time
wtpr: first 388008 1696
wtpr: second 2545147 266
wtpr: nth 3107775 235
pruc: first 28080+381451 408
pruc: second 231239+6153682 287
pruc: nth 231005+6154390 274
prc: first 0 575
prc: second 0 365
prc: nth 0 363

i'm on mac and this is my nvim version

NVIM v0.8.0-dev
Build type: Release
LuaJIT 2.1.0-beta3

update: here are the results for with this pr after a couple of hours

cache size

106103  lvim_luacache_modpaths
3446049 lvim_luacache_chunks

startuptime

257.299  000.002: --- NVIM STARTED ---
261.131  000.002: --- NVIM STARTED ---
263.070  000.002: --- NVIM STARTED ---
243.790  000.002: --- NVIM STARTED ---
259.039  000.002: --- NVIM STARTED ---
259.509  000.002: --- NVIM STARTED ---

@kylo252
Copy link
Collaborator Author

kylo252 commented Apr 19, 2022

thanks for all the effort doing all those tests! so far the results seem to indicate a 10% loss in "reported" startup time, but we still get a better profiler, some other neat tricks with overloading loadfile, and hopefully get rid of all the remaining errors generated by gitsigns.

btw, have you made some script to automate this? otherwise I can try to adapt this one https://github.com/echasnovski/mini.nvim/tree/main/benchmarks/starter

@abzcoding
Copy link
Member

hopefully get rid of all the remaining errors generated by gitsigns.

🤞

btw, have you made some script to automate this? otherwise I can try to adapt this one https://github.com/echasnovski/mini.nvim/tree/main/benchmarks/starter

no it was a manual process 😅

@kylo252
Copy link
Collaborator Author

kylo252 commented Apr 19, 2022

Here are my results with my daily config, using hyperfine with 50 rounds

Command Mean [ms] Min [ms] Max [ms] Relative
lvim 76.2 ± 1.0 74.0 78.7 1.11 ± 0.02
lv2 73.5 ± 1.1 71.5 76.1 1.07 ± 0.02
lv_no_cache 105.2 ± 1.4 102.3 109.1 1.54 ± 0.03
nvim 68.5 ± 1.1 65.7 71.1 1.00

the lv2 (e.g. ~/.local/bin/lv2) command is basically another instance of lvim with this branch

lv2
#!/usr/bin/env bash

export LUNARVIM_RUNTIME_DIR="/tmp/lv2/lunarvim"
export LUNARVIM_CONFIG_DIR="/tmp/lv2/config"
export LUNARVIM_CACHE_DIR="/tmp/lv2/cache"
export LUNARVIM_BASE_DIR="$LUNARVIM_RUNTIME_DIR/lvim"

# function install() {
#   local url="https://raw.githubusercontent.com/lunarvim/lunarvim/master/utils/installer/install.sh"
#   export LV_REMOTE="kylo252/lunarvim.git"
#   export LV_BRANCH="upstream-impatient"
#   bash <(curl -s "$url")
#   # NOTE: remember to use the same config
# }

# install

function lvim() {
  nvim -u "$LUNARVIM_BASE_DIR/init.lua" "$@"
}

lvim "$@"

the lv_no_cache (e.g. ~/.local/bin/lv_no_cache) command is basically another instance of lvim with this branch but the setup function turned off with en environment variable

diff --git a/lua/lvim/bootstrap.lua b/lua/lvim/bootstrap.lua
index d409e73..d0608f6 100644
--- a/lua/lvim/bootstrap.lua
+++ b/lua/lvim/bootstrap.lua
@@ -93,7 +93,7 @@ function M:init(base_dir)
   end
 
   -- FIXME: currently unreliable in unit-tests
-  if not in_headless then
+  if not in_headless and not os.getenv "DISABLE_IMPATIENT" then
     _G.PLENARY_DEBUG = false
     require("lvim.impatient").setup {
       chunks = { path = join_paths(get_cache_dir(), "lvim_luacache_chunks") },
benchmark.sh
#!/usr/bin/env bash
set -eo pipefail

results_dir="nv-benchmarks"

# Number of rounds to perform benchmark
n_rounds=50

# Path to output .csv file with startup times per round
csv_file="startup-times.csv"

# Path to output file with summary table
summary_json="startup-summary.json"
summary_file="startup-summary.md"

tmp_bench_file="$(mktemp)"

nvim_bins="lvim,lv2,nvim"

function benchmark {
  rm -rf "$results_dir"
  mkdir -p "$results_dir"

  pushd "$results_dir"

  local after="tail -n 1 $tmp_bench_file | cut -d ' ' -f1 >> $csv_file"
  local cmd="{nvim_bin} +'lua vim.schedule(function() vim.cmd [[qall]] end)' --startuptime $tmp_bench_file && $after"
  local setup="rm -f $csv_file && echo {nvim_bin} > $csv_file"
  # shellcheck disable=2155
  local cleanup="mv $csv_file {nvim_bin}_$csv_file"
  hyperfine --runs "$n_rounds" \
    --setup "$setup" \
    --export-json "$summary_json" \
    --export-markdown "$summary_file" \
    --cleanup "$cleanup" \
    --warmup 3 \
    --command-name "{nvim_bin}" \
    --parameter-list nvim_bin "$nvim_bins" "$cmd"

  popd
}

benchmark

@kylo252
Copy link
Collaborator Author

kylo252 commented Apr 20, 2022

@abzcoding, here are more tests using your own config on an M1 machine

image

the lv3 (e.g. ~/.local/bin/lv3) command is basically another instance of lvim with this branch impatient-v3, which incorporates a new workaround for handling the cache dir, see kylo252@8ccca67, which seems to give the best results out of all three implementations!

lv2
#!/usr/bin/env bash

export LUNARVIM_RUNTIME_DIR="/tmp/lv3/lunarvim"
export LUNARVIM_CONFIG_DIR="/tmp/lv3/config"
export LUNARVIM_CACHE_DIR="/tmp/lv3/cache"
export LUNARVIM_BASE_DIR="$LUNARVIM_RUNTIME_DIR/lvim"

function install() {
  local
  url="https://raw.githubusercontent.com/lunarvim/lunarvim/master/utils/installer/install.sh"
  export LV_REMOTE="kylo252/lunarvim.git"
  export LV_BRANCH="impatient-v3"
  bash <(curl -s "$url")

  # NOTE: remember to use the same config
  rm -rf $LUNARVIM_CONFIG_DIR
  gh repo clone abzcoding/lvim $LUNARVIM_CONFIG_DIR

  mkdir -p $LUNARVIM_CACHE_DIR
}

# install

function lvim() {
  nvim -u "$LUNARVIM_BASE_DIR/init.lua" "$@"
}

lvim "$@"

@abzcoding
Copy link
Member

fantastic, let's merge the third implementation then 👍

@kylo252
Copy link
Collaborator Author

kylo252 commented Apr 20, 2022

fantastic, let's merge the third implementation then 👍

should I move the vim.fn.stdpath change into a separate PR? wdyt?

@abzcoding
Copy link
Member

fantastic, let's merge the third implementation then 👍

should I move the vim.fn.stdpath change into a separate PR? wdyt?

let's keep it in the same PR, it is gonna be one big PR, but at least all changes related to impatient would be in the same place

@abzcoding
Copy link
Member

not sure why, but it seems like it doesn't recognize the installed lang servers
Screen Shot 2022-04-20 at 2 33 08 PM

@kylo252
Copy link
Collaborator Author

kylo252 commented Apr 20, 2022

let's keep it in the same PR, it is gonna be one big PR, but at least all changes related to impatient would be in the same place

I think the CI errors are related to how tmp files are handled on macos, could you try the branch and run the tests locally?

@kylo252
Copy link
Collaborator Author

kylo252 commented Apr 20, 2022

not sure why, but it seems like it doesn't recognize the installed lang servers

It's because it's now using an independent vim.fn.stdpath, you need to install them again or move them manually. We could also the lsp-install directory to match the one used before ~/.local/share/nvim/lsp_servers

@abzcoding
Copy link
Member

I think the CI errors are related to how tmp files are handled on macos, could you try the branch and run the tests locally?

this env var is not available on mac machines
local xdg_config = os.getenv "XDG_CONFIG_HOME"

@abzcoding
Copy link
Member

not sure why, but it seems like it doesn't recognize the installed lang servers

It's because it's now using an independent vim.fn.stdpath, you need to install them again or move them manually. We could also the lsp-install directory to match the one used before ~/.local/share/nvim/lsp_servers

we should probably do that, since the language server binaries are exactly the same for nvim and lvim

@kylo252 kylo252 changed the title fix: update impatient refactor: update impatient Apr 20, 2022
Copy link
Member

@abzcoding abzcoding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@kylo252 kylo252 merged commit 0481ec8 into LunarVim:rolling Apr 20, 2022
@kylo252 kylo252 deleted the upstream-impatient branch April 20, 2022 11:22
tomazursic pushed a commit to tomazursic/LunarVim that referenced this pull request Apr 27, 2022
* upstream/rolling: (48 commits)
  refactor: lock new installations to nvim v0.7+ (LunarVim#2526)
  fix(readme): remove black as linter (LunarVim#2510)
  fix(nvimtree): make sure on_config_done is using the correct require (LunarVim#2509)
  feat(installer): ensure correct responses when prompting user (LunarVim#2506)
  feat(peek): make sure max width and height are customizable (LunarVim#2492)
  chore: bump plugins version (LunarVim#2459)
  fix(nvimtree): escape the dot character in custom filter (LunarVim#2493)
  refactor: update impatient (LunarVim#2477)
  feat: lock nvim <0.7 to a specific tag (LunarVim#2491)
  fix(luasnip): only use user snippets if the folder exists (LunarVim#2481)
  fix(lualine): color theme gaps in some components (LunarVim#2465)
  fix(cmp): bring back default keybindings (LunarVim#2470)
  fix(cmp): update nvim-cmp to the latest version (LunarVim#2467)
  fix(readme): update lsp server ignore syntax
  refactor(lsp): cleanup servers' override configuration (LunarVim#2243)
  feat(cmp): documentation is deprecated in favor of window.documentation (LunarVim#2461)
  docs(windows): use alpha in config_win.example.lua (LunarVim#2452)
  chore: bump plugins version (LunarVim#2448)
  fix(impatient): avoid get_options in fast handler (LunarVim#2451)
  fix(luasnip): make sure all snippets are loaded (LunarVim#2447)
  ...
tomazursic pushed a commit to tomazursic/LunarVim that referenced this pull request Apr 27, 2022
* upstream/master: (63 commits)
  chore: update changelog
  refactor: lock new installations to nvim v0.7+ (LunarVim#2526)
  fix(readme): remove black as linter (LunarVim#2510)
  fix(nvimtree): make sure on_config_done is using the correct require (LunarVim#2509)
  feat(installer): ensure correct responses when prompting user (LunarVim#2506)
  feat(peek): make sure max width and height are customizable (LunarVim#2492)
  chore: bump plugins version (LunarVim#2459)
  fix(nvimtree): escape the dot character in custom filter (LunarVim#2493)
  chore: update changelog
  refactor: update impatient (LunarVim#2477)
  feat: lock nvim <0.7 to a specific tag (LunarVim#2491)
  fix(luasnip): only use user snippets if the folder exists (LunarVim#2481)
  fix(lualine): color theme gaps in some components (LunarVim#2465)
  fix(cmp): update nvim-cmp to the latest version (LunarVim#2467) (LunarVim#2469)
  fix(cmp): bring back default keybindings (LunarVim#2470)
  fix(cmp): hotfix nvim-cmp version
  fix(cmp): update nvim-cmp to the latest version (LunarVim#2467)
  chore: update changelog
  fix(readme): update lsp server ignore syntax
  refactor(lsp): cleanup servers' override configuration (LunarVim#2243)
  ...
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.

module 'gitsigns.diff_int' not found [Feature]: Use a dedicated cache directory
2 participants