Developer forums (C::B DEVELOPMENT STRICTLY!) > CodeCompletion redesign
Clang CC
l_inc:
yvesdm3000
--- Quote ---As long as it doesn't give me trouble I keep it as-is, it does help to keep the lifetime of the clang TU correct without additional pointers.
--- End quote ---
I think it's rather about avoiding additional copying of the TranslationUnit data. I'm not much into C++, and this was something new to me. It's just important to remember to copy-construct objects from temporary objects only. This includes initialization and assignment as well, because the assignment constructor of TranslationUnit gets the assigned object by value and hence will corrupt it during copy-construction.
Thing is... the compiler should do copy elision for temporary objects (which includes the m_Files vector) anyway, so it should be the same speed when doing the standard colon-initialization, but without introducing hazardous conditions for normal objects. So I would really want to know the opinion of the author of this Achtung.
yvesdm3000:
--- Quote from: l_inc on November 11, 2015, 01:14:16 am ---yvesdm3000
I think it's rather about avoiding additional copying of the TranslationUnit data. I'm not much into C++, and this was something new to me. It's just important to remember to copy-construct objects from temporary objects only. This includes initialization and assignment as well, because the assignment constructor of TranslationUnit gets the assigned object by value and hence will corrupt it during copy-construction.
--- End quote ---
When it's not C++11, the elements in the vector<TranslationUnit> will always be copy-constructed, they will then always be copied again when the vector grows beyond the allocated memory. Now for the clang TU, there is only 1 call to release that memory and when you have multiple copies of that same pointer, doing a clang_releaseTranslationUnit would then be done on all copies and hence freeing the same memory multiple times. Before C++11 we would have solved this with a pointer to TranslationUnit and do memory-management ourselves, with C++11 with the move operator it becomes much easier since the lifetime can be tied to the lifetime of the container elements, and with Alpha's trick of emulating the move operator it seems to perform the same task. It works, so I don't touch it.
Yves
Alpha:
Yes, the constructor is not intended specifically for performance (though if it helps with that, any speed is welcome), but to try to make memory management as automatic as possible: simply adding to/removing from m_TranslUnits handles the necessary operations. (Clang TU's use a lot of RAM, so leaking one is... undesirable.)
--- Quote from: l_inc on November 10, 2015, 08:22:54 pm ---I hope those who wrote the copy-constructor of the TranslationUnit as simulating a move-contructor for speed know what they do, cause it might become unsafe.
--- End quote ---
Casting away a const and editing it is undefined if the original object was created const. However, TranslationUnits must be mutable to interact with clang, so this is not the case. For insurance though, I tried to do the minimum possible operations: 3 pointer copies from the vector swap(), 1 pointer copy for the clang TU.
The self swap() of m_Files frees any over allocated memory. Since the ctor is the only place this vector is modified, it did not make sense to continue holding on to the memory.
Alpha:
Regarding RemoveTranslationUnit(), keep in mind that multiple editors for different files may utilize the same TU. Also, they are expensive to (re)create, so keeping them around (as memory constraints allow) after closing an editor helps smooth the workflow for people who frequently (re)open and close files.
It was my intent to update CreateTranslationUnit() to treat m_TranslUnits as a cache, allowing it to discard TU's after a certain limit (probably using a variant of LRU).
yvesdm3000:
--- Quote from: Alpha on November 11, 2015, 09:05:29 am ---Regarding RemoveTranslationUnit(), keep in mind that multiple editors for different files may utilize the same TU.
--- End quote ---
I think I commented-out that capability. Every file has its own TU, mostly for simplicity for now and I'm unsure if I would ever want to re-enable that, given that an include-file might look different with defines set... My focus right now is to have the basic functionality working: have CodeCompletion when you need it, no hangs but without any optimizations regarding resources and thus making many copies across threads.
I think it's even better to cache the TU to disk as a sort-of PCH when the associated file is closed. Symbol search in closed files would then reopen each PCH.
Yves
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version