This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE347038: Introduce shard storage to auto-index. (authored
by kadircet, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D54269?vs=174215&id=174339#toc
Repository:
rCTE Clang Tools E
kadircet updated this revision to Diff 174215.
kadircet marked 13 inline comments as done.
kadircet added a comment.
- Address comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
Files:
clangd/CMakeLists.txt
clangd/index/Background.cpp
clangd/index/Background.h
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Thanks! Looks good, rest of the nits are obvious or up to you.(
Comment at: clangd/index/Background.cpp:37
-BackgroundIndex::BackgroundIndex(Context BackgroundContext
kadircet updated this revision to Diff 174187.
kadircet added a comment.
- Change factory design to use llvm::unique_function.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
Files:
clangd/CMakeLists.txt
clangd/index/Background.cpp
clangd/index/Background.h
clangd/
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346938: Introduce shard storage to auto-index. (authored by
kadircet, committed by ).
Herald added a subscriber: llvm-comm
kadircet marked 9 inline comments as done.
kadircet added a comment.
I fear we still need to expose createDiskStorage, because someone still needs
to tell factory to which creator function to use.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
_
kadircet updated this revision to Diff 174052.
kadircet added a comment.
Herald added a subscriber: mgorny.
- Address comments.
- Move storage related things to new files.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
Files:
clangd/CMakeLists.txt
clangd/index/Backgro
sammccall added a comment.
just some nits. Main thing is the LoggingStorage - again I think this may have
been a misunderstanding.
Comment at: clangd/index/Background.cpp:76
+else
+ elog("Error while reading shard {0}: {1}", ShardIdentifier,
+ I.takeError())
kadircet updated this revision to Diff 174019.
kadircet added a comment.
- Get rid off getIndexStorage and use IndexStorageCreator directly.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
Files:
clangd/index/Background.cpp
clangd/index/Background.h
unittests/clangd/
kadircet updated this revision to Diff 174017.
kadircet marked 10 inline comments as done.
kadircet added a comment.
- Address comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
Files:
clangd/index/Background.cpp
clangd/index/Background.h
unittests/clangd/Back
kadircet added inline comments.
Comment at: clangd/index/Background.h:39
+
+ virtual llvm::Expected
+ loadShard(llvm::StringRef ShardIdentifier) const = 0;
sammccall wrote:
> sammccall wrote:
> > docs
> Hmm, we're going to attempt to load the shard correspondin
sammccall added a comment.
Thanks, mostly interface tweaks now.
Comment at: clangd/index/Background.cpp:187
+ BackgroundIndexStorage *IndexStorage = getIndexStorage(Directory);
+ if (!IndexStorage)
+elog("No index storage for: {0}", Directory);
I think it
kadircet added inline comments.
Comment at: clangd/index/Background.cpp:52
+llvm::SmallString<128>
+getShardPathFromFilePath(llvm::SmallString<128> ShardRoot,
+ llvm::StringRef FilePath) {
sammccall wrote:
> nit: these micro-optimizations
kadircet updated this revision to Diff 173859.
kadircet marked 13 inline comments as done.
kadircet added a comment.
- Address comments.
- Deleted shard loading logic from backgroundindex.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
Files:
clangd/index/Background.cpp
sammccall added inline comments.
Comment at: clangd/index/Background.cpp:52
+llvm::SmallString<128>
+getShardPathFromFilePath(llvm::SmallString<128> ShardRoot,
+ llvm::StringRef FilePath) {
nit: these micro-optimizations with SmallString s
kadircet added inline comments.
Comment at: clangd/index/Background.h:39
+ retrieveShard(llvm::StringRef ShardIdentifier, FileDigest Hash) const = 0;
+ virtual bool initialize(llvm::StringRef Directory) = 0;
+};
kadircet wrote:
> sammccall wrote:
> > kadircet w
kadircet updated this revision to Diff 173824.
kadircet marked 18 inline comments as done.
kadircet added a comment.
- Address comments.
- Move loading from cache out of indexing.
- Use a factory for creation of index storage.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
kadircet added inline comments.
Comment at: clangd/index/Background.h:39
+ retrieveShard(llvm::StringRef ShardIdentifier, FileDigest Hash) const = 0;
+ virtual bool initialize(llvm::StringRef Directory) = 0;
+};
sammccall wrote:
> kadircet wrote:
> > sammccall
sammccall added inline comments.
Comment at: clangd/index/Background.cpp:252
+
+auto Hash = FilesToUpdate.lookup(Path);
+// Put shards into storage for subsequent use.
kadircet wrote:
> sammccall wrote:
> > nit: i'd suggest doing the writes *after* updati
kadircet added inline comments.
Comment at: clangd/index/Background.cpp:252
+
+auto Hash = FilesToUpdate.lookup(Path);
+// Put shards into storage for subsequent use.
sammccall wrote:
> nit: i'd suggest doing the writes *after* updating the index, as the
sammccall added inline comments.
Comment at: clangd/index/Background.cpp:252
+
+auto Hash = FilesToUpdate.lookup(Path);
+// Put shards into storage for subsequent use.
nit: i'd suggest doing the writes *after* updating the index, as the latter is
user-fa
kadircet updated this revision to Diff 173281.
kadircet marked an inline comment as done.
kadircet added a comment.
- clang-format
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
Files:
clangd/index/Background.cpp
clangd/index/Background.h
unittests/clangd/Background
Eugene.Zelenko added inline comments.
Comment at: clangd/index/Background.cpp:29
#include
+#include
+#include
Please run Clang-format. Headers in sections should be in alphabetical order.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
kadircet created this revision.
kadircet added reviewers: sammccall, ioeric.
Herald added subscribers: cfe-commits, arphaman, jkorous, ilya-biryukov.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
Files:
clangd/index/Background.cpp
clangd/index/Background.h
unittests/
24 matches
Mail list logo
