This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Use AsyncTaskRunner in BackgroundIndex instead of std::thread
ClosedPublic

Authored by ilya-biryukov on May 9 2019, 4:23 AM.

Event Timeline

ilya-biryukov created this revision.May 9 2019, 4:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 9 2019, 4:23 AM
kadircet accepted this revision.May 9 2019, 4:36 AM

LGTM

clang-tools-extra/clangd/index/Background.cpp
152

NIT: why not count from zero :P

164

Destructor of AsyncTaskRunner already does that

This revision is now accepted and ready to land.May 9 2019, 4:36 AM
ilya-biryukov marked 4 inline comments as done.
  • Count from 0.
clang-tools-extra/clangd/index/Background.cpp
152

Done. The worker numbers still start with 1, sorry about that :-)

164

Yeah, but then we'll have to ensure the AsyncTaskRunner is the last member (I've been bitten by this before).
Having an explicit call to wait() in the destructor is bulletproof.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 9 2019, 5:03 AM
umanwizard added inline comments.
clang-tools-extra/trunk/clangd/index/Background.cpp
164 ↗(On Diff #198795)

This is already called in ~AsyncTaskRunner.

Ignore me. I didn't read the previous discussion before commenting.