[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous added a comment. We did performance tests of alternative approach - just hashing the serialized bit code representation. There's a performance regression in the sense that while the current implementation costs approx. extra 2.2% in build time the alternative approach costs 3.8%. We

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous added a comment. In D58749#1426769 , @kadircet > In D58749#1426778 , @gribozavr > I see what you mean now. That's a good idea. I'll add some unit tests. CHANGES SINCE LAST ACTION

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous marked 8 inline comments as done. jkorous added a comment. Addressed some comments, going to update the diff. Comment at: clang/lib/Index/IndexRecordHasher.cpp:291 + +hash_code IndexRecordHasher::hashImpl(const Decl *D) { + return DeclHashVisitor(*this).Visit(D);

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous updated this revision to Diff 190342. jkorous marked 2 inline comments as done. jkorous added a comment. Addressed some of Dmitri's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58749/new/ https://reviews.llvm.org/D58749 Files: clang/lib/Index/CMakeLists.txt

[PATCH] D58749: [index-while-building] IndexRecordHasher

gribozavr added a comment. > I basically didn't really like the idea of testing against hard-coded hash > values in unittests as I consider it to be an implementation detail. Sorry, that's not what I was suggesting. There are better ways to test hashing. For example, write a piece of source

[PATCH] D58749: [index-while-building] IndexRecordHasher

kadircet added a comment. > I basically didn't really like the idea of testing against hard-coded hash > values in unittests as I consider it to be an implementation detail. I was > thinking about integration tests that would work around this by both writing > index and reading index and

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous added a comment. In D58749#1426270 , @gribozavr wrote: > I left some comments, but it is difficult for me to review without > understanding what the requirements for this hasher are, why some information > is hashed in, and some is left out,

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous updated this revision to Diff 190317. jkorous added a comment. Herald added a subscriber: jfb. Based on Kadir comment I refactored the code. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58749/new/ https://reviews.llvm.org/D58749 Files:

[PATCH] D58749: [index-while-building] IndexRecordHasher

kadircet added inline comments. Comment at: clang/lib/Index/IndexRecordHasher.h:41 + /// Returns hash for all declaration occurences in \c Record. + llvm::hash_code hashRecord(const FileIndexRecord ); + llvm::hash_code hash(const Decl *D); Why expose hashing

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous added reviewers: kadircet, gribozavr. jkorous added a comment. Adding clangd folks in case they want to take a look. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58749/new/ https://reviews.llvm.org/D58749

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous created this revision. jkorous added reviewers: nathawes, akyrtzi, arphaman, dexonsmith, ioeric, malaperle. Herald added subscribers: cfe-commits, jdoerfert, mgorny. Herald added a project: clang. Another piece of index-while-building functionality. RFC: