infer Incremental build support for json-based frontend

Incremental build support for json-based frontend

Open xi-liu-ds opened this issue 2 years ago • 13 comments

Hi!

In this PR we add incremental build support for json-based frontend. All we need is provide a list of changed files in a txt file and feed in through flag --changed-files-index for infer analyzejson command.

Please let us know your thoughts. Thanks!

Feb 01 '22 22:02 xi-liu-ds

Hi @xi-liu-ds, sorry for the delay. It looks like this duplicates more code still from InferAnalyze.ml. Would that duplication still be needed if parsing the json was done in a capture phase instead of together with the analysis? It feels to me like doing so would give you incremental analysis for free as well as being more maintainable, as the analysis phase would be just the normal Infer analysis. Also there wouldn't need to be an analyzejson command.

I think capturing json could be done similarly as how we capture clang compilation databases so you could take inspiration from those code paths. We use it like this:

# does capture + analysis
$ infer run --compilation-database file.json
# equivalent to running these two commands:
$ infer capture --compilation-database file.json
$ infer analyze

Similarly we'd have two options --json-cfg and --json-tenv that have to be specified together or not at all:

# does capture + analysis
$ infer run --json-cfg cfg.json --json-tenv tenv.json
# equivalent to running these two commands:
$ infer capture --json-cfg cfg.json --json-tenv tenv.json
$ infer analyze

So we would have a new driver mode JsonSIL set similarly to ClangCompilationDB. Then it's just a matter of separating out the capture phase out of InferAnalyzeJson.ml and into its own module and call it from Driver.ml, then the analysis phase can be deleted from InferAnalyzeJson.ml (and so the rest of the file with it) as it's not a separate thing from the normal infer analysis anymore.

Would that approach work for you and are there any blockers? I'm not sure how much we need to ensure backwards compatibility or we can just get rid of the analyzejson command altogether if we go down this route, for example.

Feb 11 '22 16:02 jvillard

Hi @xi-liu-ds, sorry for the delay. It looks like this duplicates more code still from InferAnalyze.ml. Would that duplication still be needed if parsing the json was done in a capture phase instead of together with the analysis? It feels to me like doing so would give you incremental analysis for free as well as being more maintainable, as the analysis phase would be just the normal Infer analysis. Also there wouldn't need to be an analyzejson command.

I think capturing json could be done similarly as how we capture clang compilation databases so you could take inspiration from those code paths. We use it like this:
# does capture + analysis
$ infer run --compilation-database file.json
# equivalent to running these two commands:
$ infer capture --compilation-database file.json
$ infer analyze
Similarly we'd have two options --json-cfg and --json-tenv that have to be specified together or not at all:
# does capture + analysis
$ infer run --json-cfg cfg.json --json-tenv tenv.json
# equivalent to running these two commands:
$ infer capture --json-cfg cfg.json --json-tenv tenv.json
$ infer analyze
So we would have a new driver mode JsonSIL set similarly to ClangCompilationDB. Then it's just a matter of separating out the capture phase out of InferAnalyzeJson.ml and into its own module and call it from Driver.ml, then the analysis phase can be deleted from InferAnalyzeJson.ml (and so the rest of the file with it) as it's not a separate thing from the normal infer analysis anymore.

Would that approach work for you and are there any blockers? I'm not sure how much we need to ensure backwards compatibility or we can just get rid of the analyzejson command altogether if we go down this route, for example.

Thanks Jules for the suggestion! We are aware that analyze json has some code duplication from InferAnalyze.ml. We are investigating the solution you provided and see if it can fully replace the current experience from InferAnalyzeJson.ml.

Feb 11 '22 17:02 xi-liu-ds

Hi @jvillard So sorry about the delay on this PR. I finally had chance to look deep into it this week 😂.

It turns out that your suggestions are great and helped to remove lots of duplicate code! Thank you so much!

Like you suggested, now we can do infer capture --cfg-json cfg.json --tenv-json tenv.json to run json frontend analysis. And for incremental build, all we need is infer capture --cfg-json cfg.json --tenv-json tenv.json --incremental-analysis --changed-files-index change_file_index.txt

Please take a look when you get a chance. Thanks again!

Jul 01 '22 23:07 xi-liu-ds

Thanks for your comments! Now the incremental build support has been improved even more. It can be triggered by using the following commands:

infer run --cfg-json cfg.json --tenv-json tenv.json --incremental-analysis --changed-files-index change_file_index.txt -j 1

Jul 12 '22 23:07 xi-liu-ds

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