isakbm / gitgraph.nvim

Git Graph plugin for neovim
MIT License
309 stars 7 forks source link

Crash & hang on big repos #1

Closed iguanacucumber closed 3 months ago

iguanacucumber commented 3 months ago

on the rust repo, it crashes:

E5108: Error executing lua: ...er/.local/share/nvim/lazy/gitgraph.nvim/lua/gitgraph.lua:461: attempt to 
index local 'next_commit' (a nil value)                                                                 
stack traceback:                                                                                        
        ...er/.local/share/nvim/lazy/gitgraph.nvim/lua/gitgraph.lua:461: in function 'get_is_bi_crossing
'                                                                                                       
        ...er/.local/share/nvim/lazy/gitgraph.nvim/lua/gitgraph.lua:711: in function 'straight_j'       
        ...er/.local/share/nvim/lazy/gitgraph.nvim/lua/gitgraph.lua:838: in function 'gitgraph'         
        ...er/.local/share/nvim/lazy/gitgraph.nvim/lua/gitgraph.lua:2029: in function 'draw'            
        /home/user/.config/nvim/init.lua:486: in function </home/user/.config/nvim/init.lua:485>        
Press ENTER or type command to continue

and on the linux kernel repo, it just hangs and locks neovim entirely

isakbm commented 3 months ago

Thanks for opening the first issue!

Could you provide links to the repositories that the plugin is struggling with?

isakbm commented 3 months ago

I just ran it on the rust repo, and I had no issues other than that it took a good 5 seconds to complete.

image

It is slow but it works.

Performance is something that I will be improving asap of course :)

Could you link the repo that is causing the crash for you?

iguanacucumber commented 3 months ago

https://github.com/rust-lang/rust -> crash the plugin https://github.com/torvalds/linux -> crash neovim / softlock Is there any log i can provide ?

isakbm commented 3 months ago

The most helpful would be for me to try to reproduce the error on my side. The crash in the rust repo is not something I am able to reproduce at the moment.

What arguments / options are you passing to "gitgraph.nvim" draw ?

iguanacucumber commented 3 months ago

i orriginaly tried with this: require('gitgraph').draw({}, { all = true, max_count = 5000 }) then i tried with or without some arguments, but it still crash every time i do a require

isakbm commented 3 months ago

Can you try with max_count = 10?

EDIT: I just tried with max_count = 10, and I get

E5108: Error executing lua: ...rs/isak/code/nvim-plugins/gitgraph.nvim/lua/gitgraph.lua:461: attempt to ind
ex local 'next_commit' (a nil value)
stack traceback:
        ...rs/isak/code/nvim-plugins/gitgraph.nvim/lua/gitgraph.lua:461: in function 'get_is_bi_crossing'
        ...rs/isak/code/nvim-plugins/gitgraph.nvim/lua/gitgraph.lua:711: in function 'straight_j'
        ...rs/isak/code/nvim-plugins/gitgraph.nvim/lua/gitgraph.lua:838: in function 'gitgraph'
        ...rs/isak/code/nvim-plugins/gitgraph.nvim/lua/gitgraph.lua:2030: in function 'draw'

So thats good 🎉

Means that I can resolve this next_commit error. And seems that it is unrelated to the size of the repo.

Will look into this. :)

isakbm commented 3 months ago

For now can you try with max_count = 100, just out of curiosity. This seems to at least on my end to not trigger the bug. Hoping that you have the same behaviour on your end. That is, I hope that if you set max_count=100, that it works.

Of course that is not a solution, just a curiosity that will give me an idea of how severe this bug is or not.

iguanacucumber commented 3 months ago

same error

iguanacucumber commented 3 months ago

i tried the plugin again, on macos this time and it works on the rust repo ( it just takes 2 whole minutes, and uses 2G of RAM but it works )

isakbm commented 3 months ago

Thanks for this added information.

The memory usage is expected to be high as literally the entire history is loaded into memory in lua tables.

Pagination (buffering) etc will be one of the first optimisations I carry out. Hopefully it will reduce memory consumption and speed up the rendering / loading :)

Good news is that the graph drawing algo is local and deterministic so it can run on partial log data making it easy to optimize.

isakbm commented 3 months ago

Most likely found the cause of the crash.

Will aim for a patch within 24 hours. 🤞

Performance optimisation will be an ongoing project that will most likely take another couple of weeks though.

isakbm commented 3 months ago

@iguanacucumber

The crash should now hopefully be fixed.

Performance is still an issue of course, but will eventually be worked on, will therefore close this issue, and open a new one that is purely focused on performance.

iguanacucumber commented 3 months ago

it works 🥳