[Lldb-commits] [PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Chuanqi Xu via Phabricator via lldb-commits]

ChuanqiXu added a comment.

In D126694#4470235 <https://reviews.llvm.org/D126694#4470235>, @iains wrote:

> In D126694#4470139 <https://reviews.llvm.org/D126694#4470139>, @ChuanqiXu wrote:
>
>> Now I think the feature may be important for the performance of modules. And I feel we should work on the ASTWriter side instead of ASTReader side. Since the size of BMIs is a problem now also it shows that it is not free to load the large BMIs. So while it is semantical correct to work on the reader side, it is better for the performance to work on the writer side.
>>
>> I'd like to finish the idea. And for the current patch, I'd like to refactor it a little bit:
>>
>> 1. Test it by unittest instead of by matching the dump result.
>
> fine with me
>
>> 2. Remove the Serialization part. So it will be a NFC patch.
>
> how do you plan to identify "elided" decls (we agreed to leave them in the BMI to keep diagnostics quality up, but we need to be able to ignore them)

No. Now I feel it is not good to keep these redundant things in the BMI. They slow down  the performance. The enlarge the size of the BMI. They make the model more complex. They've brought a lot of pain points but they bring few benefits. I really don't think we should keep them. And this is the reason why I re-looked at this.

>> 3. Some other trivial polishment.
>
>
>
> 4. I wanted to take another look at using visitors to implement the checks.

Yeah, this is what I called polishment.

>> Of course, I'll still mark you as the author.
>>
>> How do you feel about this?
>
> Do you plan to try this before clang-17?
>  (if so, then go ahead - my next priority for 17 is the lookup bug)
>
> Otherwise, this is on my TODO for clang-18.

I want to give it a try for clang-17. The direct motivation is that I found the performance of the std module is worse than the direct #include in a small program (~20 lines). After many analysising, I think this one may be the most important technique to improve that. But it is only possible if we elide them in the BMI when writing. It doesn't help if we still keep them in the BMI.

And I feel it pretty bad. This is the reason why I want to make it into clang-17.

> BTW, I think that there are other opportunities to reduce the BMI size that we could realistically aim for clang-18
>  (but let us make that discussion separately from this patch)

Yeah. I tried to look at it a little bit but it looks not so straight forward. A main part I found now is the source managers in serializer.  But let's discuss it separately.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126694/new/

https://reviews.llvm.org/D126694