This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Add removeFile interface in FileIndex.
Needs ReviewPublic

Authored by hokein on Oct 8 2018, 3:35 AM.

Details

Reviewers
ioeric
sammccall
Summary

Currently we don't cleanup dynamic index if a file is closed in clangd,
which may result in large memory consumption in clangd (if we open many files
and closes them).

This patch is the first step.

Event Timeline

hokein created this revision.Oct 8 2018, 3:35 AM
hokein updated this revision to Diff 168638.Oct 8 2018, 3:36 AM

clang-format.

ioeric added inline comments.Oct 8 2018, 4:04 AM
clangd/index/FileIndex.h
78

should we use this somewhere? E.g. when file is closed in ClangdServer?

hokein added inline comments.Oct 8 2018, 5:14 AM
clangd/index/FileIndex.h
78

Yes, the usage of this method is not included in this patch.

We probably use this function in TUScheduler::remove, the code path will be like

TUScheduler::remove => ParsingCallback::onASTRemove => FileIndex::removeFile.

We need to add a similar interface to ParsingCallback.

ioeric added inline comments.Oct 11 2018, 4:55 AM
clangd/index/FileIndex.h
78

Or maybe in ClangdServer::removeDocument?

hokein added inline comments.Oct 11 2018, 8:02 AM
clangd/index/FileIndex.h
78

It seems that we will have problems if calling it in ClangdServer::removeDocument.

When removing a file, we schedule a remove task in TUScheduler, and the AST thread will wait for all pending tasks to be finished and then exit.

An example like:

  1. The AST thread has a pending request of update AST task (which will update the FileIndex) in its task queue.
  2. We remove the documentation, FileIndex will be cleanup immediately in ClangdServer::removeDocument, and clangd schedules a remove task.
  3. The AST thread processes all pending tasks (including update AST), and exits, Now the symbol comes back again.