[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D101793#2772033 <https://reviews.llvm.org/D101793#2772033>, @weiwang wrote:

> In D101793#2772021 <https://reviews.llvm.org/D101793#2772021>, @yaxunl wrote:
>
>> In D101793#2769297 <https://reviews.llvm.org/D101793#2769297>, @weiwang wrote:
>>
>>> Tried to make `Sema::DeclsToCheckForDeferredDiags` `llvm::SmallSetVector`. The heap RSS did drop significantly (from peak 100GB to 59GB) , but not as good as the current fix (peak 26GB), which makes `ASTReader::DeclsToCheckForDeferredDiags` `llvm::SmallSetVector`.
>>>
>>> I think the reason is that the duplicated decls are read from multiple module file sources (`ASTReader::ReadAST()` -> `ASTReader::ReadASTBlock()`), then stored into `ASTReader::DeclsToCheckForDeferredDiags`, then goes into `Sema::DeclsToCheckForDeferredDiags` in `ASTReader::ReadDeclsToCheckForDeferredDiags()`. Doing dedup at the early stage when the decls were just read in `ASTReader` is more effective at reducing RSS.
>>
>> What if you use SmallSetVector for both Sema::DeclsToCheckForDeferredDiags and ASTReader::DeclsToCheckForDeferredDiags? Does it cause extra memory usage compared to using it only for ASTReader::DeclsToCheckForDeferredDiags? Thanks.
>
> There would be a slight increase in memory usage since SmallSetVector requires more memory than just SmallVector internally, but given the majority the RSS comes from duplicated decls, I don't think it's an issue by making both SmallSetVector. If you think it's better to change both, I'll update the diff.

Let's use SmallSetVector for both. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101793/new/

https://reviews.llvm.org/D101793