mpark wrote:

> > The patch itself looks good. But I like to test it internally before 
> > merging. Hope I can make it in next week.
> 
> 
> 
> In our internal workloads, I didn't see noticeable changes in End-to-End 
> build process. Maybe the reason is we've already done a so called "bottom up" 
> build. But the good news is that I didn't find regression too. So LGTM.

Thanks for checking! Yeah, unless you have fine-grained modules with 1K+ 
modules declaring the same namespaces, I wouldn't expect to see any 
improvements with this.

For us, it's because at a certain scale with Thrift generated files, we end up 
with 1K+ modules imported that all define the `apache::thrift` namespace.

> 
> 
> 
> > 
> 
> > And also I think it is worthy to add a test for it. We are able to test it 
> > with unittests. e.g., we can test the number of the return decls of 
> > `DeclContext::lookup`, or other similar words. We can add some API for 
> > testing such status too.
> 
> 
> 
> Would you like to add a test for this?  If you don't know how to do, you can 
> look at clang/unittests/Serialization/LoadSpecLazilyTest.cpp for example. 
> This is useful as lit test can't show all things.
>

I'll be working on the test tomorrow! Thanks for the pointer.


https://github.com/llvm/llvm-project/pull/171769
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to