search

lewis6991 / impatient.nvim

Improve startup time for Neovim
MIT License
1.19k stars 28 forks source link

break!: split apart module path and chunk cache #46

Closed lewis6991 closed 2 years ago

lewis6991 commented 2 years ago

Notice:

If this PR causes any breakage for anyone, please raise an issue and roll back to the latest release with the tag v0.1.

lewis6991 commented 2 years ago

@clason any chance you could help me test this? This PR is almost a complete rewrite by splitting the cache into two components.

The main logic is in place, just need to work on the profiling reporting some more.

clason commented 2 years ago

Gladly! I tested an earlier version, and it seemed to work fine (although my startup time increased slightly; I assume that my config is too simple for the small overhead to pay off).

clason commented 2 years ago

Just to make sure: that needs your Neovim PR?

lewis6991 commented 2 years ago

Nope, but this PR will automagically work with it.

clason commented 2 years ago

Seems to work fine; I briefly put it through the usual motions and noticed no regressions. I did see the same increase in startup time (~30ms to 32ms). I could add the profiles for both versions, if that helps.

Should I test with the Neovim PR as well? Should that improve the timings?

lewis6991 commented 2 years ago

Seems to work fine; I briefly put it through the usual motions and noticed no regressions. I did see the same increase in startup time (~30ms to 32ms). I could add the profiles for both versions, if that helps.

Depends how you are measuring. I tweaked the profiler quite a bit and the difference between 30ms and 32ms is probably in the margin of error.

The new code could be a little bit slower as it needs to run a couple of gsub's to reconstruct module names, but in the scheme of things it shouldn't make much difference. We could avoid this if we really want to, but I don't think it's absolutely necessary as it would introduce a bit more complexity.

Should I test with the Neovim PR as well? Should that improve the timings?

With my plugin setup, there were only like 5 paths that where loaded that weren't done through require, and I think most of these files are juse one-liners with a singlerequire so the caching doesn't help too much. So for now, I would say the Neovim PR adds negligible benefit, however this could change if more plugins use the plugin and after directories more.

The main benefit of this PR is that it caches both require and loadfile. I remember at some point lsp-config was using loadfile to avoid the module resolution cost of require.

Also, since this PR doesn't add any loaders to package.loaders it will speed up statements like pcall(require, 'noexist'), since there are less loaders to traverse.

clason commented 2 years ago

Depends how you are measuring. I tweaked the profiler quite a bit and the difference between 30ms and 32ms is probably in the margin of error.

I use https://github.com/tweekmonster/startuptime.vim

The sign of the delta is pretty consistent, though (but the magnitude is indeed not considerable).

With my plugin setup, there were only like 5 paths that where loaded that weren't done through require, and I think most of these files are juse one-liners with a singlerequire so the caching doesn't help too much. So for now, I would say the Neovim PR adds negligible benefit, however this could change if more plugins use the plugin and after directories more.

For me, the biggest "offenders" are probably lsp setup, snippet definitions, and my colorscheme (color/ is cached as well, right?)

The main benefit of this PR is that it caches both require and loadfile. I remember at some point lsp-config was using loadfile to avoid the module resolution cost of require.

Makes sense. lspconfig does not, but telescope does, once or twice.

Also, since this PR doesn't add any loaders to package.loaders it will speed up statements like pcall(require, 'noexist'), since there are less loaders to traverse.

That is a pattern that is used quite a bit, especially in telescope.

lewis6991 commented 2 years ago

I use tweekmonster/startuptime.vim

The sign of the delta is pretty consistent, though (but the magnitude is indeed not considerable).

OK, I'd attribute it to the extra gsubs then. To avoid them we'd need to add a loader, or, replace vim._load_package. I'll do that in this PR so we can confirm that is the regression.

For me, the biggest "offenders" are probably lsp setup, snippet definitions, and my colorscheme (color/ is cached as well, right?)

It would be with that PR, the impatient profile would confirm. Additional non-modules paths are reported at the bottom in a separate table.

clason commented 2 years ago

It would be with that PR, the impatient profile would confirm. Additional non-modules paths are reported at the bottom in a separate table.

Hmm, I don't see it in the profile. I don't see any of my lua files in .config/nvim -- do I need to set something up differently for that? I just switched the branch and didn't mess with the loading of impatient.

(I do see a trouble which I don't remember installing... EDIT: Oh, that's probably from gitsigns, which tries to pcall it.)

lewis6991 commented 2 years ago

Let me investigate.

clason commented 2 years ago

Ah, there we go -- I wasn't on your Neovim branch (mumblemumble stupid gh).

I now see those Lua files in the impatient profile, but the startuptime log still shows them taking the full time as without the PR?

Probably loading simple Lua files is simply dominated by executing the bytecode, so the savings are at best in the single percent and eaten by the added overhead. But I guess performance improvements are not the main point of these two PRs, right?

lewis6991 commented 2 years ago

If impatient profiler is showing them, with the cache loader, then surely it is working? If the time is the same, doesn't that just mean the caching didn't provide much uplift?

lewis6991 commented 2 years ago

Cold run:

────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Files loaded with no associated module
────────────┬───────────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       Time │    Loader │ Path
────────────┼───────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
    0.169ms │  standard │ <STD_CONFIG>/colors/github_dark.lua
    0.102ms │  standard │ <PACKER>/start/nvim-cmp/plugin/cmp.lua
    0.073ms │  standard │ <PACKER>/start/cmp-buffer/after/plugin/cmp_buffer.lua
    0.048ms │  standard │ <PACKER>/start/cmp_luasnip/after/plugin/cmp_luasnip.lua
    0.040ms │  standard │ <PACKER>/start/cmp-treesitter/after/plugin/cmp_treesitter.lua
    0.033ms │  standard │ <VIMRUNTIME>/filetype.lua
    0.028ms │  standard │ <PACKER>/start/cmp-rg/after/plugin/cmp-rg.lua
    0.028ms │  standard │ <PACKER>/start/cmp-cmdline/after/plugin/cmp_cmdline.lua
    0.026ms │  standard │ <PACKER>/start/cmp-emoji/after/plugin/cmp_emoji.lua
    0.025ms │  standard │ <PACKER>/start/cmp-nvim-lua/after/plugin/cmp_nvim_lua.lua
    0.023ms │  standard │ <PACKER>/start/cmp-spell/after/plugin/cmp-spell.lua
    0.023ms │  standard │ <PACKER>/start/cmp-path/after/plugin/cmp_path.lua
    0.022ms │  standard │ <PACKER>/start/cmp-nvim-lsp-signature-help/after/plugin/cmp_nvim_lsp_signature_help.lua
    0.021ms │  standard │ <PACKER>/start/cmp-nvim-lsp/after/plugin/cmp_nvim_lsp.lua
────────────┴───────────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

Cached run:

────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Files loaded with no associated module
────────────┬───────────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       Time │    Loader │ Path
────────────┼───────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
    0.025ms │     cache │ <STD_CONFIG>/colors/github_dark.lua
    0.014ms │     cache │ <PACKER>/start/nvim-cmp/plugin/cmp.lua
    0.005ms │     cache │ <PACKER>/start/cmp-buffer/after/plugin/cmp_buffer.lua
    0.005ms │     cache │ <PACKER>/start/cmp-cmdline/after/plugin/cmp_cmdline.lua
    0.005ms │     cache │ <PACKER>/start/cmp-nvim-lsp-signature-help/after/plugin/cmp_nvim_lsp_signature_help.lua
    0.005ms │     cache │ <PACKER>/start/cmp-nvim-lsp/after/plugin/cmp_nvim_lsp.lua
    0.005ms │     cache │ <PACKER>/start/cmp-emoji/after/plugin/cmp_emoji.lua
    0.005ms │     cache │ <PACKER>/start/cmp-path/after/plugin/cmp_path.lua
    0.004ms │     cache │ <VIMRUNTIME>/filetype.lua
    0.004ms │     cache │ <PACKER>/start/cmp_luasnip/after/plugin/cmp_luasnip.lua
    0.004ms │     cache │ <PACKER>/start/cmp-rg/after/plugin/cmp-rg.lua
    0.004ms │     cache │ <PACKER>/start/cmp-treesitter/after/plugin/cmp_treesitter.lua
    0.004ms │     cache │ <PACKER>/start/cmp-spell/after/plugin/cmp-spell.lua
    0.004ms │     cache │ <PACKER>/start/cmp-nvim-lua/after/plugin/cmp_nvim_lua.lua
────────────┴───────────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

So for me at least this is making a huge difference... relatively.

clason commented 2 years ago

For me it's

Cold:

────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Files loaded with no associated module
────────────┬───────────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       Time │    Loader │ Path
────────────┼───────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
    0.390ms │  standard │ <STD_CONFIG>/colors/moonberg.lua
    0.208ms │  standard │ <STD_CONFIG>/plugin/lsp.lua
    0.179ms │  standard │ <STD_DATA>/site/pack/loader/start/packer.nvim/plugin/packer.lua
    0.136ms │  standard │ <STD_CONFIG>/plugin/luasnip.lua
    0.075ms │  standard │ <STD_CONFIG>/plugin/telescope.lua
    0.065ms │  standard │ <STD_CONFIG>/plugin/packer.lua
    0.064ms │  standard │ <STD_CONFIG>/plugin/hop.lua
    0.062ms │  standard │ <VIMRUNTIME>/filetype.lua
    0.061ms │  standard │ <STD_CONFIG>/plugin/lualine.lua
    0.058ms │  standard │ <STD_CONFIG>/plugin/gitsigns.lua
    0.044ms │  standard │ <STD_CONFIG>/plugin/comment.lua
────────────┴───────────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

Cached:

────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Files loaded with no associated module
────────────┬───────────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       Time │    Loader │ Path
────────────┼───────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
    0.248ms │     cache │ <STD_CONFIG>/plugin/packer.lua
    0.091ms │     cache │ <STD_CONFIG>/colors/moonberg.lua
    0.039ms │     cache │ <STD_DATA>/site/pack/loader/start/packer.nvim/plugin/packer.lua
    0.037ms │     cache │ <STD_CONFIG>/plugin/luasnip.lua
    0.032ms │     cache │ <STD_CONFIG>/plugin/lsp.lua
    0.023ms │     cache │ <STD_CONFIG>/plugin/telescope.lua
    0.022ms │     cache │ <STD_CONFIG>/plugin/comment.lua
    0.019ms │     cache │ <STD_CONFIG>/plugin/gitsigns.lua
    0.018ms │     cache │ <STD_CONFIG>/plugin/hop.lua
    0.015ms │     cache │ <STD_CONFIG>/plugin/lualine.lua
    0.013ms │     cache │ <VIMRUNTIME>/filetype.lua
────────────┴───────────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

So I also see some benefit in load time, but it doesn't really translate to a big difference in total self-time (strangely enough). The increase for plugin/packer.lua is also curious.

lewis6991 commented 2 years ago

Updated to replace vim._load_package again.

clason commented 2 years ago

Yeah, that seems to make a bigger difference. packer.lua now also decreases in cost.

clason commented 2 years ago

(Sorry for complicating matters; I didn't mean to derail this PR.)

lewis6991 commented 2 years ago

Yeah, that seems to make a bigger difference. packer.lua now also decreases in cost.

It shouldn't. The change to package.loaders will only affect modules loaded with require. Anything in plugin, colors, etc, won't be influenced as that simply goes through loadfile().

(Sorry for complicating matters; I didn't mean to derail this PR.)

It's been very valuable feedback.

clason commented 2 years ago

It shouldn't. The change to package.loaders will only affect modules loaded with require. Anything in plugin, colors, etc, won't be influenced as that simply goes through loadfile().

OK, then that's just flakiness on my machine -- it's an almost 5 year old iMac. (One reason I use tweekmonster's plugin, which will take the mean of N runs (usually with N=100).)

Maybe I should just take out my M1 MBA and see how it looks on that SSD ;)

clason commented 2 years ago

Yeah, basically same behavior: significant reduction in load time, but still minor (~1%) but consistent increase in startup time according to --startuptime profiling -- which quite possibly can't be trusted anymore (or even less) with these changes.

msva commented 2 years ago

Btw, this PR removed the example about caching packer_compiled from README. Is it not needed anymore, or what?

lewis6991 commented 2 years ago

It is no longer needed if your Neovim build has this change: https://github.com/neovim/neovim/pull/17200

msva commented 2 years ago

thanks a lot