rmaz added a comment.

In D109632#3022209 <https://reviews.llvm.org/D109632#3022209>, @vsapsai wrote:

> That patch looks correct, I was experimenting with exactly the same local 
> change. Have you tried D110123 <https://reviews.llvm.org/D110123> on your 
> original build? In my testing with synthetic test case it looks as good as 
> set deduplication.

I am getting some pretty unexpected results from this approach. For the files I 
could get to complete below are the compilation times for each approach for the 
10 slowest (in the baseline) files:

| **file**   | **baseline** | **set deduplication** | **no external methods in 
pcm** |
| Total      | 95.57        | 75.30                 | 306.46                    
     |
| IGSFCVC.m  | 4.57         | 1.53                  | 13.71                     
     |
| IGVFVC.mm  | 4.19         | 1.61                  | 27.93                     
     |
| IGMFAHC.mm | 4.00         | 1.93                  | 9.94                      
     |
| IGMFLCVC.m | 3.89         | 1.55                  | 12.30                     
     |
| PLRFPPC.mm | 3.19         | 3.26                  | 3.99                      
     |
| IGBFPVC.m  | 3.18         | 1.11                  | 28.92                     
     |
| IGBVC.m    | 2.37         | 0.98                  | 17.42                     
     |
| PLRRM.mm   | 1.99         | 1.99                  | 2.31                      
     |
| IGBSPSC.m  | 1.88         | 0.85                  | 14.38                     
     |
|

All of the most interesting files were left out of these results as I could not 
get compilation to complete for the no external methods approach.

> I still think it is worth to pursue writing only owned methods in 
> `METHOD_POOL`. Deduplication with a DenseSet works but I still expect it to 
> be more expensive than avoiding duplicates by construction. As for the next 
> step I was thinking about changing `ASTMethodPoolTrait` to skip transitive 
> methods during serialization and it should help with eliminating all the 
> duplicates. With that change we can test and see how it works.

I will take a go at this approach to eliminate all external methods and compare 
to see if this exhibits similar behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109632

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to