[PATCH] D124240: [clangd][NFC] Reduce memory usage while building dex
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 22 06:17:24 PDT 2022
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:89
+ Result.try_emplace(Token(Token::Kind::Trigram, E.first.str()),
+ std::move(E.second));
+ TrigramDocs.clear();
----------------
kadircet wrote:
> sammccall wrote:
> > the move here does nothing, we're passing as ArrayRef
> >
> > did you want to clear the map values (vectors) as you went, too?
> >
> > (this gets a bit verbose, but you could pull out a function template since all 4 cases are so regular)
> > did you want to clear the map values (vectors) as you went, too?
>
> yes that's the idea. would you prefer an explicit destructor call?
>
> > (this gets a bit verbose, but you could pull out a function template since all 4 cases are so regular)
>
> the problem is 1 of these is a densemap, other three is a stringmap, and onther is a vector. densemap and the stringmap doesn't really go together because there's no common method to get the key (densemap has `first` as a field, or `getFirst()` as a method. stringmap has `first()` or `getKey()` as methods :/).
> happy to move out just the three of them into a helper like:
> ```
> buildPostingList(StringMap<..>, TokenKind, Out);
> ```
> if you think that's still helpful (that way we can also get rid of the calls to clear since we can just move the maps into the function arguments).
> yes that's the idea. would you prefer an explicit destructor call?
I mean e.second.clear() in the loop.
As it stands, you're only destroying them a whole map at a time, and trigrams is (i think) >50% of the total
> the problem is 1 of these is a densemap, other three is a stringmap, and onther is a vector.
Sounds too complicated, nevermind!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124240/new/
https://reviews.llvm.org/D124240
More information about the cfe-commits
mailing list