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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
