Olical / aniseed

Neovim configuration and plugins in Fennel (Lisp compiled to Lua)
https://discord.gg/wXAMr8F
The Unlicense
606 stars 28 forks source link

Slow startup time #37

Closed Javyre closed 3 years ago

Javyre commented 3 years ago

related: #24

Hi, I'm running neovim 0.5 and have switched over my entire config to fennel with aniseed. However, I'm getting noticeable slower startup now since aniseed apparently takes around 50ms to load according to nvim --startuptime:

...
069.000  050.430  048.199: sourcing /home/javyre/.local/share/nvim/site/pack/paqs/start/aniseed/plugin/aniseed.vim
...

This ~50ms represents 50% of my now 100ms startup time. Can you reproduce this or is it something wrong in my config? I don't think its just my disk being slow as was mentioned on #24 as I'm running on an m.2 ssd as well.

I'm loading aniseed with compile=false and compiling on file save to save on startup time already (which saved me around 20ms)

Update: The same exact dotfiles synced on my laptop give me a total startup time of 260ms with aniseed taking up 195ms)

Olical commented 3 years ago

Hey thanks for reporting this! I did some work to speed it up in the past but clearly not enough. I'll take a look at benchmarking various sections to work out what's slowing thing's down soon. It could be all the extra code I need to add to each module to make interactive evaluation work nicely or maybe my aniseed.core standard library.

I was planning on replacing a lot of my core map / filter / reduce with https://github.com/luafun/luafun at some point, which may massively help alongside simplifying my module macro output (if possible...).

Since Lua / Fennel doesn't really support namespaces I got clever with my module macros, but I guess that's hitting performance at larger codebase scales (i.e. Conjure). Sotte on the Conjure discord has also measured really high load times (relatively) from Conjure.

Javyre commented 3 years ago

Thanks for the response!

It could be all the extra code I need to add to each module to make interactive evaluation work nicely

Are you talking about the fennelview stuff? I feel like requiring fennelview can be deferred to when we're actually calling serialize. It seems to get required on every module macro.

I was planning on replacing a lot of my core map / filter / reduce with https://github.com/luafun/luafun at some point, which may massively help alongside simplifying my module macro output (if possible...).

Luafun sounds great, but would this speed aniseed up? wouldn't there be more code to load with luafun?

Sotte on the Conjure discord has also measured really high load times (relatively) from Conjure.

Yeah, I had to lazy load conjure to avoid an extra ~40ms to my startup time.

I'll be looking forward to your benchmark results!

Olical commented 3 years ago

Not fennelview, no, it's the extra maps and meta data that I add to enable autocompletion + context aware evaluations. Without this there's not enough information to enable the smarter interactions you'd expect because Lua just wasn't intended for the sort of interactions Aniseed allows. I'll rethink an idea I had a while back about producing "slim" module code on disk and then upgrading it to metadata laden code when required.

I don't think it's volume of code but more the amount of work being performed while loading it. I'm pretty sure Lua can load a LOT of code very quickly, so maybe Aniseed pokes the filesystem once or twice and that's enough to slow it down. Needs some investigation, but I'm thinking speeding up my core functions I use a lot while slimming down macro output code will be the way to go.

Luafun is a very small library, I think it just has better implementations of functional lazy concepts. Might be a lot of work but I'll have a look.

Javyre commented 3 years ago

Sounds good! Let me know if there's anything I can do to test or help out!

Olical commented 3 years ago

So I added a system environment variable and Lua global called ANISEED_LIGHT which enables "light" module output in the module macros. This is the default for all AOT compiling as well as aniseed.env configuration loaded for the moment. I may regret this and roll it back if this breaks things horribly.

This is all on the develop branch of Aniseed and Conjure, you should see a difference in file size at the very least. I don't know if you'll see a significant reduction in load time though, I had a quick look and it seems to have made a difference but I'd like to get your 3rd party take on it.

The downside / tradeoff: The lua modules on disk are now regular lua modules, lacking all the magic metadata that Aniseed adds through the module macros which enables the fancy interactive evaluations. You may run into weird issues while interacting with a file through Conjure until you evaluate the file again in Conjure, enriching it with metadata.

A way around this I may add if you think this has made a significant difference: Have Conjure detect "light" modules and warn you about this caveat and prompt you to load the file through Conjure to enable interactivity.

I won't experiment any further until people confirm that this is VERY much in the right direction though. If it's saving only like 5-10% it's probably not worth the added complexity. If it's more like 60% or more, then maybe it's worth it and will require further investment.

Javyre commented 3 years ago

hmm, I switched to devel and now I'm getting:

Error detected while processing /home/javyre/.local/share/nvim/site/pack/paqs/start/aniseed/plugin/aniseed.vim:                                                                       
line    4:
E5108: Error executing lua [string "luaeval()"]:1: Vim(echoerr):E121: Undefined variable: vim

It seems to be ba6665f that causes this error. The commit just before that one works fine for me.

A way around this I may add if you think this has made a significant difference: Have Conjure detect "light" modules and warn you about this caveat and prompt you to load the file through Conjure to enable interactivity.

This seems very reasonable to me.

Olical commented 3 years ago

Hmm that one just involved updating the Fennel compiler which passed all tests πŸ€” I'll have a dig! Do you happen to be on Neovim 0.4.4 or 0.5+?

Edit: I see you mentioned 0.5, please ignore!

Olical commented 3 years ago

Not sure what I'll do to fix your error just yet (no idea why we'd see that, vim is a global within the nvim lua context) but I just had a thought: I might introduce lazy loading of requires, kind of like vim script's autoload idea. So by default all Aniseed module macros will add an invisible layer of indirection that lazy loads modules as they're actually used. This could make a HUGE difference πŸ‘€

I'll need a way to opt out for some specific reasons, but yay, could make more Aniseed based projects automatically faster! I like improving Aniseed since it improves EVERYTHING that depends on this tower of parens πŸ˜„

I think it's already a lot faster, but this should get us another jump in performance. All I really need to do is shift the load time from startup to some later user interaction.

Javyre commented 3 years ago

Alright so I poked around and I think I found where it's coming from.

This line is sending an unquoted/escaped string to the echoerr command which is trying to evaluate the error message as a vim expression: https://github.com/Olical/aniseed/blob/ac29422c024586e0f82a76c76019e4b158851eb0/fnl/aniseed/env.fnl#L12

This leads to the confusing and meaningless "vim not found" error. If I add a print(err) manually before the echoerr, I get the actual error that the fennel upgrade caused:

vim.lua:129: /home/javyre/.local/etc/nvim/lua/init.lua:282: ambiguous syntax (function call x new statement) near '('

It looks like there may have been some regression in this version of fennel... I'll do some more research and try to find it.

Edit: For the sake of completeness here's the generated code causing the error:

  local opts = {expr = true, noremap = true, silent = true}
  local _2_0 = require("util")
  (_2_0).map("i", "<C-d>", "compe#scroll({ 'delta': -4 })", opts) -- <-- LINE 209 HERE
  ; (_2_0).map("i", "<C-f>", "compe#scroll({ 'delta': +4 })", opts)
  ; (_2_0).map("i", "<C-e>", "compe#close('<C-e>')", opts)
  ; (_2_0).map("i", "<CR>", "compe#confirm('<CR>')", opts)
  ; (_2_0).map("i", "<C-Space>", "compe#complete()", opts)

And here is the generated code on aniseed:master:

  local opts = {expr = true, noremap = true, silent = true}
  local _3_0 = require("util")
  do end (_3_0).map("i", "<C-d>", "compe#scroll({ 'delta': -4 })", opts)
  do end (_3_0).map("i", "<C-f>", "compe#scroll({ 'delta': +4 })", opts)
  do end (_3_0).map("i", "<C-e>", "compe#close('<C-e>')", opts)
  do end (_3_0).map("i", "<CR>", "compe#confirm('<CR>')", opts)
  do end (_3_0).map("i", "<C-Space>", "compe#complete()", opts)
Javyre commented 3 years ago

I opened a ticket reporting the regression: https://todo.sr.ht/~technomancy/fennel/53

Javyre commented 3 years ago

I removed the 0.9 upgrade commit on my local copy and tested out the light mode anyways. I can't really tell any noticeable difference in startup time sadly :frowning_face:. I do (unsurprisingly) notice that there's less code generated though which is always a plus.

Javyre commented 3 years ago

I have to admit that halfway through all this I realized that the startuptime listing for aniseed.vim I showed in the OP includes the time of executing my own config files since aniseed.env loads them in aniseed.vim. I'm now optimizing my config and getting many good time savings by deferring lots of innocent-looking requires.

Aniseed may or may not be slow at this points I really don't know how to measure it in isolation...

Javyre commented 3 years ago

Closing this issue as I was able to greatly reduce the startup time by deferring the module requires in my config. If anyone wants to use aniseed in a "light mode", I think aniseed is very usable without the (module) and (defn) macro magic and I'm guessing that's a use case you'd like to support.

I don't think aniseed is really to blame for any major startup time issues I can see right now.

Olical commented 3 years ago

Hmm interesting! Well I'm probably going to revert the ANISEED_LIGHT stuff because it introduces two separately maintained chunks of macros that is a recipe for disaster. It didn't actually make it faster but it added a LOT of complexity for me as the maintainer as well as users.

Going to experiment with making all requires lazy (so they require at the point you look something up in them) with an optional opt out where required (shouldn't be noticeable in 99% of cases though)

Olical commented 3 years ago

Okay! All pushed! So the slight bloat to the code will be back (afraid it's either that or MASSIVE risk of bugs / reduced interactivity for weird reasons that users will get bitten by all the time), might try to reduce it again in the future but I'm not too worried about it.

I've added an autoload function that works just like require but it performs the load when you first try to use the table.

(module foo
  {require {my-mod x.y.z}})

;; becomes...

(module foo
  {autoload {my-mod x.y.z}})

I've swapped this over for every Aniseed, Conjure and nvim-local-fennel module on the develop branches of each repo. I'm actually seeing a fairly large jump and I could probably get a LOT more by deferring specific things like my personal configuration.

I don't want to add automatic deferring since that can break so many assumptions. This autoload should be completely invisible in most cases but delays the require until it's actually used. Sometimes this won't help, but I'd argue that it's worth using as a default over require until you hit an issue, then swap to require (like if you need eager loading for side effects).

I hope this helps and is interesting!

Javyre commented 3 years ago

Yeah, I actually did something similar in my config: https://github.com/Javyre/etc/blob/d4b307b933aa50ebc9f99575ba02f312e9452520/nvim/fnl/macros.fnl#L1

I find it most useful for smaller scopes like here for example where I only need to require when the keybind is called and not when it is defined: https://github.com/Javyre/etc/blob/d4b307b933aa50ebc9f99575ba02f312e9452520/nvim/fnl/init.fnl#L306

Olical commented 3 years ago

Exactly what autoload is designed for ☺️

On Sun, 25 Apr 2021, 17:03 Javier Pollak, @.***> wrote:

Yeah, I actually did something similar in my config: https://github.com/Javyre/etc/blob/d4b307b933aa50ebc9f99575ba02f312e9452520/nvim/fnl/macros.fnl#L1

I find it most useful for smaller scopes like here for example where I only need to require when the keybind is called and not when it is defined: https://github.com/Javyre/etc/blob/d4b307b933aa50ebc9f99575ba02f312e9452520/nvim/fnl/init.fnl#L306

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Olical/aniseed/issues/37#issuecomment-826347681, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACM6XPSANT6FB5AMROB2T3TKQ4NRANCNFSM43CH5Q2A .