[clang][Modules] PCM Sizes Increased Significantly After Delta Encoding Optimization Removed

[qiongsiwu]

Hi @hokein @ChuanqiXu9 !

I work with @Bigcheese and @cyndyishida on Clang Modules. We noticed that the PCM sizes have increased significantly (5%) after #145670. I can see that the 5% increase is expected, but combining this with some other commits that increase PCM sizes, we are experiencing build failures where builders run out of disk space.

Do we have a plan going forward to claw back some of the disk space increase caused by this PR?

[llvmbot]

@llvm/issue-subscribers-clang-modules

Author: Qiongsi Wu (qiongsiwu)

Hi @hokein @ChuanqiXu9 !

I work with @Bigcheese and @cyndyishida on Clang Modules. We noticed that the PCM sizes have increased significantly (5%) after #145670. I can see that the 5% increase is expected, but combining this with some other commits that increase PCM sizes, we are experiencing build failures where builders run out of disk space.

Do we have a plan going forward to claw back some of the disk space increase caused by this PR?

[shafik]

CC @rnk @AaronBallman @erichkeane

[shafik]

Can you identify what are the other commits that increase PCM size?

[shafik]

I am going to point out that we do try to take feedback on PRs pretty seriously, especially from key stakeholders but I will additionally say modules PRs usually don't get a lot of feedback outside a small set of people. I would love to see a wider set of folks regularly reviewing modules PRs and maybe this will allow us to address these types issues more effectively earlier in the process.

[qiongsiwu]

Thanks for chiming in @shafik !

Can you identify what are the other commits that increase PCM size?

d3daa3c causes 3% size increase for some of our projects. In total, these two commits are causing an 8% increase, which becomes significant when added together.

[AaronBallman]

I'm not aware of active plans to claw that memory back, but it would certainly be welcome effort to improve compile time overhead, build artifact size, etc.

[qiongsiwu]

Thanks @AaronBallman !

@hokein other than code clean up, what was the intention of #145670? Was it preparing for enabling 64 bit source location?

[ChuanqiXu9]

We noticed that the PCM sizes have increased significantly (5%) after #145670. I can see that the 5% increase is #145670 (comment), but combining this with some other commits that increase PCM sizes, we are experiencing build failures where builders run out of disk space.

The problem here is, we thought 5% increase in size is slight but you think it is significant. The direct motivation is, we feel the complexity of the code is hard to read and maintain while we don't get significant benefits from it (we don't feel 5% size of PCMs is significant). @hokein experienced this when he did some changes for 64 bit source location. And I also met similar problems while I was refactoring the encodings for source location. And I feel it will be a problem too when we try to implement other refactorings. So from the perspective of maintainability, I think it is better to remove the optimization.