clang's CXTranslationUnit_PrecompiledPreamble madness

[Sergey Kalinichev]

Hi,

> Clang's C API won't actually create a preamble if you don't call clang_reparseTranslationUnit after the initial clang_parseTranslationUnit2 call...

Yes, that's a known bug, which was recently fixed:
http://reviews.llvm.org/D15490
And I actually think that we should use that newly introduced flag
(when available), instead of parsing the same file twice.

--
Sergey


On 1/10/16, Kevin Funk <kfunk at kde.org> wrote:
> On Saturday, January 09, 2016 06:28:02 PM Milian Wolff wrote:
>> Hey all, esp. Sergey, Kevin and Olivier.
>>
>> After quite some time I finally sat down today to write a proper
>> benchmark
>> for our clang-based code completion. It's relatively slow for big C++
>> include chains as we all know. It's still bearable most of the time where
>> it's in the ~250ms space on my machine, but definitely noticeable and of
>> course worse on slower machines.
>>
>> Now during the recent Qt World Summit I chatted with Marco Bubke who's
>> nowadays working on Qt Creator's clang code model. One nugget he shared
>> with
>> me is that Clang's C API won't actually create a preamble if you don't
>> call
>> clang_reparseTranslationUnit after the initial
>> clang_parseTranslationUnit2
>> call...
>>
>> With my new benchmark you can see the impact this has, on a KDev* build
>> without any optimizations:
>>
>> no preamble:
>>
>> RESULT : BenchCodeCompletion::benchCodeCompletion():"empty":
>>      2.4 msecs per iteration (total: 79, iterations: 32)
>> --
>> RESULT : BenchCodeCompletion::benchCodeCompletion():"stl":
>>      171 msecs per iteration (total: 171, iterations: 1)
>> --
>> RESULT : BenchCodeCompletion::benchCodeCompletion():"clib":
>>      66 msecs per iteration (total: 66, iterations: 1)
>>
>> with preamble:
>>
>> RESULT : BenchCodeCompletion::benchCodeCompletion():"empty":
>>      1.7 msecs per iteration (total: 55, iterations: 32)
>> --
>> RESULT : BenchCodeCompletion::benchCodeCompletion():"stl":
>>      9.7 msecs per iteration (total: 78, iterations: 8)
>> --
>> RESULT : BenchCodeCompletion::benchCodeCompletion():"clib":
>>      18 msecs per iteration (total: 74, iterations: 4)
>>
>> So from this I think it's pretty clear that we want this. The reason I
>> write
>> this lengthy mail instead of just committing
>
> Thanks for the benchmarks. Numbers are always good.
>
>> diff --git a/languages/clang/duchain/parsesession.cpp b/languages/clang/
>> duchain/parsesession.cpp
>> index f992a1c..e6c7385 100644
>> --- a/languages/clang/duchain/parsesession.cpp
>> +++ b/languages/clang/duchain/parsesession.cpp
>> @@ -237,6 +237,7 @@ ParseSessionData::ParseSessionData(const
>> QVector<UnsavedFile>& unsavedFiles, Cla
>>      }
>>
>>      if (m_unit) {
>> +        clang_reparseTranslationUnit(m_unit, unsaved.size(),
>> unsaved.data(), clang_defaultReparseOptions(m_unit));
>>          setUnit(m_unit);
>>          m_environment = environment;
>>
>> is that it breaks quite some unit tests, even with Clang 3.7.0:
>>
>> FAIL!  : TestCodeCompletion::testVariableScope() Compared values are not
>> the
>> same
>>    Actual   (tester.items.size()): 3
>>    Expected (4)                  : 4
>>    Loc:
>> [/home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/languages/
>> clang/tests/test_codecompletion.cpp(988)]
>> FAIL!  : TestCodeCompletion::testCompleteFunction(add-parens) Compared
>> values are not the same
>>    Actual   (view->document()->text()): "int foo();\nint main()
>> {\nmain()\n}" Expected (expectedCode)            : "int foo();\nint
>> main()
>> {\nfoo()\n}" Loc:
>> [/home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/languages/
>> clang/tests/test_codecompletion.cpp(1100)]
>> FAIL!  : TestCodeCompletion::testCompleteFunction(keep-parens) Compared
>> values are not the same
>>    Actual   (view->document()->text()): "int foo();\nint main()
>> {\nfoo();\n}" Expected (expectedCode)            : "int foo();\nint
>> main()
>> {\nmain();\n}" Loc:
>> [/home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/languages/
>> clang/tests/test_codecompletion.cpp(1100)]
>> FAIL!  : TestDUChain::testReparseUnchanged(template-default-parameters)
>> 'implCtx->problems().isEmpty()' returned FALSE. ()
>>    Loc:
>> [/home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/languages/
>> clang/tests/test_duchain.cpp(1397)]
>>
>> So I think it looks like we'll have to dig into Clang to fix these issues
>> and then commit the patch above. Anyone interested in helping out? Also
>> if
>> you have access to older or newer clang, please try the above and see
>> what
>> it breaks.
>
> The testCompleteFunction is flaky for me, very likely timing related.
>
> I had runs with zero failures, then with 1, and sometimes with the 3
> failures
> you get there.
>
>> Furthermore, note that we already do generate a preamble file if the user
>> edits the file! So the above test issues are affecting our users *now*.
>> Maybe we want to commit the patch after all?
>
> The test case from above:
>
> int foo();
> int main()
> {
>     <complete here>
>     return 0;
> }
>
> ... works fine for me *with* your patch in a normal KDevelop session. I get
>
> (foo, main) as completion items, if I just press enter I get the desired
> result: foo().
>
> I guess its just our tests which are broken...?
>
> Am I missing something?
>
> Cheers,
> Kevin
>
> --
> Kevin Funk | kfunk at kde.org | http://kfunk.org