ycm-core / YouCompleteMe

A code-completion engine for Vim
http://ycm-core.github.io/YouCompleteMe/
GNU General Public License v3.0
25.45k stars 2.81k forks source link

Limit number of cached TUs in YCMD (feature request) #2232

Closed mrbiggfoot closed 5 years ago

mrbiggfoot commented 8 years ago

As I described here, YCMD memory usage on my project renders the plugin useless. I don't care much about precise completion engine, but the semantically correct GoTo command is what makes my life enormously easier.

Is it possible to implement a feature to keep only one TU (or, more generic, a fixed number of TUs) in memory? An LRU cache of TUs seems like a feasible solution. It would help to keep YCMD memory usage sane, and I'm fine with having to wait while the TU is being compiled.

Thanks!

puremourning commented 8 years ago

We just merged a change which significantly reduces ram usage. Can you try it out?

On 6 Jul 2016, at 08:54, Andrew Pyatkov notifications@github.com wrote:

As I described here, YCMD memory usage on my project renders the plugin useless. I don't care much about precise completion engine, but the semantically correct GoTo command is what makes my life enormously easier.

Is it possible to implement a feature to keep only one TU (or, more generic, a fixed number of TUs) in memory? An LRU cache of TUs seems like a feasible solution. It would help to keep YCMD memory usage sane, and I'm fine with having to wait while the TU is being compiled.

Thanks!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

micbou commented 8 years ago

@puremourning It only affects the memory usage from identifiers, not the TUs.

mrbiggfoot commented 8 years ago

@puremourning, I already did - unfortunately, it does not help at all.

oblitum commented 8 years ago

Associating information: https://github.com/Valloric/YouCompleteMe/issues/1844#issuecomment-170715347

Valloric commented 8 years ago

This doesn't strike me as an unreasonable feature request. It would necessitate another YCM user-level option though, which I loathe (we have way too many). But it might be worth it.

The option would be something like g:ycm_max_cfamily_cached_translation_units, with -1 as the default meaning no limit. We'd have an LRU tracking open c-family buffers. But it might be non-trivial work which depending on how much code it would be, might not be worth it. If we add the feature, we'll be supporting it forever. Also, we've done dandy fine without it for years...

@puremourning @micbou @vheon Thoughts?

vheon commented 8 years ago

Wouldn't we trade, as always, memory consumption with speed?

oblitum commented 8 years ago

@vheon it would be a user's job to trade, YCM by itself would be agnostic I guess. As I referred in https://github.com/Valloric/YouCompleteMe/issues/1844#issuecomment-170715347, besides the implementation of providing an option with the actual number of TUs, there's also one based on memory usage limit, on python in another plugin I made use of psutils for that, so the number of TUs was variable until reaching a limit of memory usage.

At the time quite some years back, I didn't notice problems while coding, I guess it happened because I (or probably most people) won't be actually making code edits to several files at once, switching TUs like crazy back and forth, where it would be noticeable. A TU would be dropped at the time I've opened the 5th or 10th one, in FIFO fashion.

Anyways, this would not help when a single TU, like when including Boost.Bimap, consumed ridiculous amounts of memory without means of parsing to occur partially.

mrbiggfoot commented 8 years ago

@vheon, yes and no. Yes - up to a certain point. No - when memory is all consumed by YCMD, the OS starts swapping heavily. At this point even the basic processes like terminals are slowed down to crawl. I'd rather experience some delays in auto-completion than deal with OS freezes.

BTW, I hit a new record recently. One .cc and one .h ate 7.5G of physical memory in YCMD. Forgive me, but I refuse to believe that there are no bugs. :)

oblitum commented 8 years ago

@mrbiggfoot check https://github.com/Valloric/YouCompleteMe/issues/1844. Your problem may be lying outside ycmd help.

mrbiggfoot commented 8 years ago

@oblitum, I did not say the bug is necessarily in YCMD.

oblitum commented 8 years ago

@mrbiggfoot OK. But it's also a possibility that there may be no bugs and it's just the amount of memory libclang requires for your TU.

oblitum commented 8 years ago

If libclang offered introspection of how much memory more or less it would require for parsing a TU, absurd usages that would lead to swapping could be avoided. Dunno whether there are any means for doing that.

oblitum commented 8 years ago

Ah yeah, there's a not so pretty option of compiling AST to disk first and check file size, and then decide whether it should be compiled to memory.

oblitum commented 8 years ago

Also, I recall reading libclang docs in remote times where it allowed loading AST from disk into memory, without having to parse two times, or having precompiled headers ASTs... but my memory may be failing. I've seen no plugin using libclang for completion using this kind of compile-AST-to-disk and load-AST-from-disk feature.

oblitum commented 8 years ago

http://clang.llvm.org/doxygen/group__CINDEX__TRANSLATION__UNIT.html#ga0659baf7f04381286ec54b439760c8f3

oblitum commented 8 years ago

I guess that trying to make use of that would change how YCMD works a lot.

puremourning commented 8 years ago

@oblitum, I did not say the bug is necessarily in YCMD

If there is only 1 TU open and using 7GB RAM, then limiting the number of TUs wont help. And as you say, if indeed there is a bug, it is in libclang.

So I do t really see how we can help here. I suggest you concoct a reduced test case (such as the bitmap one) and report to cfe-dev.

If it is discovered that there is indeed a YCM or ycmd bug, then we will gladly fix it.

mrbiggfoot commented 8 years ago

As everyone has said, TUs can be very large. Not limiting their number and letting YCMD consume all the memory in the world is simply a bad design. If you are serious about this project, it's got to be fixed regardless of whether there is a bug in clang. I will try to find a minimal repro, but honestly it's very low priority in my task list. Now I rest my case.

Valloric commented 8 years ago

Not limiting their number and letting YCMD consume all the memory in the world is simply a bad design. If you are serious about this project, it's got to be fixed regardless of whether there is a bug in clang. I will try to find a minimal repro, but honestly it's very low priority in my task list. Now I rest my case.

Responses like this one aren't a very good way of achieving your goals.

You have implied we aren't serious about this project, that we're bad at prioritizing work, that you spending time to repro your issue isn't worth your time (but should be worth ours, implying your time is much more valuable than ours) and then you closed it off by implying you've said everything necessary and we should just go do what you've said.

Please consider your words more carefully in the future otherwise you risk not being taken seriously and ignored.

mrbiggfoot commented 8 years ago

@Valloric, you got me wrong.

You have implied we aren't serious about this project,

I have not. What I meant is that perhaps the problem was not obvious at the time when you were implementing YCMD, but now it is. And if you are serious about the project, it needs to be fixed at some point. Ok, perhaps my standards of software design are too high, but I was under impression that yours are too, if not even higher. It's pretty sad that I have to prove an obvious point that memory usage has to be capped.

that we're bad at prioritizing work

I did not say that.

that you spending time to repro your issue isn't worth your time (but should be worth ours, implying your time is much more valuable than ours)

What I said is that I got a lot of tasks that are more important to me that reproducing a clang issue. I did not say that you have to repro it. I meant leaving clang bugs to clang devs, but making sure YCMD behaves sane in the corner cases.

and then you closed it off by implying you've said everything necessary and we should just go do what you've said.

I indeed implied that I said everything necessary. Whether you do it or not is obviously up to you, and scaring me off by saying that I risk not being taken seriously simply does not work. I already got rid of YCM in my neovim setup (replacing it with deoplete + ctags + neomake - I know it's not as good, but still works), so I've got nothing to lose.

Please don't try to find offense in my words. I respect people spending their personal time to make open source products and fully understand that there are no obligations on their side. What I'm trying to discuss here is a bug in the product that I used to like. I see a lot of potential in YCM, that's why I'm wasting your and my time trying to suggest how to make it better. Let's leave feelings aside and talk logic. And, once again, I don't intend - and never have - to put any offense in my words.

oblitum commented 8 years ago

@mrbiggfoot

Let's leave feelings aside and talk logic. And, once again, I don't intend - and never have - to put any offense in my words.

  • Reduce your issue in a test case, preferably separating YCMD using too much memory vs libclang doing it.
  • YCMD using too much memory should be fixed in YCMD. libclang using too much memory, which has been said already, can happen with a single TU, if that's the case:
  • Suggest what's a solution for that, because as has been said, limiting TU count won't help, which is your original suggestion.
Valloric commented 8 years ago

I'm locking this thread to collaborators only to prevent the discussion going further off base. This is something we want to improve and will when we find the time to do it.

Thank you to everyone for sharing their opinion.

micbou commented 5 years ago

As mentioned in https://github.com/Valloric/YouCompleteMe/issues/2232#issuecomment-236040484, the suggestion doesn't help in the case of a single TU. For multiple TUs, the memory used by one TU can be freed by deleting the corresponding buffer. I think this is good enough. Closing as won't be implemented.