[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. I've decided to land this without the test. Thank you for your contribution. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101763/new/ https://reviews.llvm.org/D101763 ___

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

This revision was automatically updated to reflect the committed changes. Closed by commit rGdb8af0f21dc9: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand… (authored by OikawaKirie, committed by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie added a comment. In D101763#2777821 , @steakhal wrote: > It seems like everything passes. Yeey, good job! > Shall I commit this tomorrow on your behalf? I do not have commit access to the code base. It would be appreciated if you could

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal accepted this revision. steakhal added a comment. It seems like everything passes. Yeey, good job! Shall I commit this tomorrow on your behalf? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101763/new/ https://reviews.llvm.org/D101763

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie updated this revision to Diff 347446. OikawaKirie added a comment. Re-submit the patch to re-build the code. Sorry for the spam of submitting an incomplete diff which triggers a patch apply failure. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101763/new/

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie updated this revision to Diff 347441. OikawaKirie added a comment. Re-submit the patch to re-build the code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101763/new/ https://reviews.llvm.org/D101763 Files: clang/include/clang/CrossTU/CrossTranslationUnit.h

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. Could you please reupload your diff to retrigger the bots? I'd like to make sure that everything passes, even the tests that do not belong to you. Unfortunately, I don't know any better way of retriggering the pre-merge checks - I don't have permission to manually

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie updated this revision to Diff 347327. OikawaKirie added a comment. Herald added a subscriber: manas. In D101763#2768549 , @martong wrote: > First of all, thank you for the patch! > We had a meeting with my colleges (@steakhal, @gamesh411) and

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

martong added a comment. First of all, thank you for the patch! We had a meeting with my colleges (@steakhal, @gamesh411) and we agreed in the following. This is indeed an issue and the fix is okay. About the test, we'd like to ask if you could create a small test lib with its own 'open'

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie added a comment. > FileCheck error: '' is empty. It fails again on the build bot, another dead end. Maybe I need a Debian server to reproduce the problem encountered in the test case. In D101763#2745918 , @steakhal wrote: > In

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. In D101763#2745802 , @OikawaKirie wrote: > I do not know why the test case always fails on the build server, it runs > perfectly on my computer. : ( I've locally run your patch and seemed good to me, I regret not having a

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie updated this revision to Diff 343818. OikawaKirie added a comment. Monitor function `open` together with `openat` to fix the failure in the test case. As some distros actually call function `open` rather than forward to function `openat`, both functions should be monitored by

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie added a comment. In D101763#2741629 , @steakhal wrote: > Okay, so you 'just' want an indication for the given open call. What about > using `strace`? > `strace -e trace=openat %clang_cc1 ... 2>&1 | grep '"invocations.yaml"' | > FileCheck

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I would love to see `PreviousParsingResult` combined with `InvocationList` using a `llvm::Error`. I'm pretty sure it can be done. Regardless, I think it's already better than it was.

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie updated this revision to Diff 343626. OikawaKirie added a comment. In D101763#2743984 , @steakhal wrote: > By inspecting the output that is piped to the `count`, I have a suspicion: > > openat(AT_FDCWD, "invocations.yaml",

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. I would rather match on the complete line (except the file descriptor ofc). By inspecting the output that is piped to the `count`, I have a suspicion: openat(AT_FDCWD, "invocations.yaml", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory) It should have

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie updated this revision to Diff 343585. OikawaKirie added a comment. Replace `wc -l` with `count`. Sorry for the spam. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101763/new/ https://reviews.llvm.org/D101763 Files: clang/include/clang/CrossTU/CrossTranslationUnit.h

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie updated this revision to Diff 343582. OikawaKirie added a comment. - Update the test case as suggested. - Add a case branch for the `index_error_code::success` error code in function `IndexErrorCategory::message` to silent the compiler warnings. CHANGES SINCE LAST ACTION

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. Okay, so you 'just' want an indication for the given open call. What about using `strace`? `strace -e trace=openat %clang_cc1 ... 2>&1 | grep '"invocations.yaml"' | FileCheck %s` This way the given test file could contain the contents of the `importer.c`. CHANGES

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie updated this revision to Diff 343346. OikawaKirie added a comment. In D101763#2741536 , @steakhal wrote: > Overall, it looks promising. But I don't quite get this test. > There is no invocation yaml in the temp directory. So, you are probably

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. Overall, it looks promising. But I don't quite get this test. There is no invocation yaml in the temp directory. So, you are probably not testing the right thing. You wanted to test if the invocation yaml exists, and could be opened **but the parsing fails**. You

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie updated this revision to Diff 343293. OikawaKirie added a comment. Add a regression test case by mocking the `open` function. When this function is called with the file name of the invocation list, the mocked `open` function will reject the open operation and dump a log. And we will

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

balazske added inline comments. Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:257 llvm::Optional InvocationList; +index_error_code InvocationListParsingError = index_error_code::no_error; }; martong wrote: > I think it would be more

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

balazske added inline comments. Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:670 return llvm::Error::success(); + else if (index_error_code::no_error != InvocationListParsingError) +return llvm::make_error(InvocationListParsingError); martong

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

martong added a comment. Thank you for this patch! Could you please provide a lit test that ignites the over and over parsing behaviour? I think you need to create two files and the second one should contain parser error(s). Comment at:

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. Awesome! Seems good to me. Though I've got limited experience on CTU stuff. It would be nice to have tests, but it seems pretty hard to come up with one for this. Given that this is just a 'performance' issue, I'm fine with it. Somehow try to check if this resolved your

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

OikawaKirie created this revision. OikawaKirie added reviewers: gamesh411, balazske, martong, xazax.hun. OikawaKirie added a project: clang. Herald added subscribers: steakhal, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.