alexfh added a comment.

In D128113#3751477 <https://reviews.llvm.org/D128113#3751477>, @mizvekov wrote:

> In D128113#3747946 <https://reviews.llvm.org/D128113#3747946>, @alexfh wrote:
>
>> For now we've added a workaround for the most problematic use case and got 
>> us unblocked. Thus there is no need now in introducing additional flags. 
>> However, I'm rather concerned about the cumulative additional overhead our 
>> build infrastructure gets as a result of this patch. Measuring this would 
>> unfortunately be a project of its own, which I'm not ready to commit to. If 
>> reverting this and discussing with Richard Smith is an option for you, I'd 
>> suggest doing this now and if you end up not finding a good alternative, we 
>> would probably need to find a way to estimate the impact of this patch. WDYT?
>
> Thanks, and sorry for the long time to respond to this.
>
> I have been thinking about this problem, and I have an idea for a solution 
> which is better on the performance side, but more complex change.
>
> We would need to implement some pattern matching during AST traversal, and 
> keep track of the pack index there. When extracting a component of the larger 
> type, such as when deducing non-pack arguments from a pack, we would wrap it 
> over with a new sugar type node which encodes the current pack indexes.
>
> I reflected a bit on this, and yeah I think we should try to design something 
> which does not potentially hinder large packs so much.
>
> I am not sure how this is going to play out in practice, and it's possible we 
> may need to come back to this solution again, but I think it's prudent to 
> revert it for now if we are not sure, as this is an API change, however 
> obscure for most users.

I appreciate your effort on designing an alternative solution. Hopefully, it 
works out well.

> As an aside, it would be really helpful if you could track how the 
> compilation of your codebase performs as clang evolves, regardless if we keep 
> this change or not :)
> Maybe you can reuse something from http://llvm-compile-time-tracker.com/

Good point about tracking compile time. We do have compiler performance tests 
running in a controlled environment with a number of measures in place to 
reduce errors in results, but they are limited to a rather small sample of 
build targets. Tracking compiler performance (times, memory used) for all of 
our code and using this as a regression test for compiler performance is 
impractical due to the size of the code base, rate of changes, and the design 
of the build system (high level of parallelism, aggressive caching, shared 
build cluster consisting of machines with different performance 
characteristics).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128113

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

Reply via email to