Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow concurrent indexing in glean index cpp-cmake #271

Closed

Conversation

rleshchinskiy
Copy link
Contributor

This adds support for concurrent indexing based on the existing worklist framework. There are some shenanigans
related to progress reporting which mostly exist to make indexing LLVM nicer (script coming up).

The other major addition is the index-llvm.sh script which automates indexing LLVM source distributions using the parallel indexer - try the example suggested in index-llvm.sh --help. I'm not sure where the script should live. On my server indexing takes about 10 min with -j16.

Note that this includes #269 - we don't have stacked PRs so that one should probably be landed first.

This should be landed together with facebookincubator/hsthrift#102 which removes ffi.h from fb-util
This is necessary to support parallel indexing in the `cpp-cmake` CLI.
It's a bit of a hack but seems to be the best way of doing this at the
moment since we want to be able to obtain this from LLVM CDBs.
This uses the existing worklist support. There are some shenanigans
related to progress reporting which mostly exist to make indexing LLVM
nicer (script coming up).

Tested with -j16 for indexing LLVM.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 11, 2022
@netlify
Copy link

netlify bot commented Sep 11, 2022

Deploy Preview for fb-oss-glean canceled.

Name Link
🔨 Latest commit 4e2a6bf
🔍 Latest deploy log https://app.netlify.com/sites/fb-oss-glean/deploys/632018cd07565f000862a59b

This runs the indexer on the LLVM source distribution. I think it's a
good demo. `index-llvm.sh --help` gives a complete example. I'm not sure
where the script should live, though.
@facebook-github-bot
Copy link
Contributor

@donsbot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

This removes all remaining missing header file errors.
Copy link
Contributor

@alpmestan alpmestan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (just spotted a typo)

index-llvm.sh Outdated
dir=$(dirname "${dir}")
done
if [[ -z "${GLEAN_DIR}" ]] ; then
fatal "Couldn't local Glean source directory, please specify --glean"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/local/locate/

@facebook-github-bot
Copy link
Contributor

@donsbot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@donsbot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@pepeiborra has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

glean.cabal.in Show resolved Hide resolved
@rleshchinskiy rleshchinskiy deleted the rl-cpp-index-dev branch October 15, 2022 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants