[libcxx-commits] [PATCH] D144568: [libc++][NFC] Refactor the __enable_ifs in <string>

[Alexander Kornienko via Phabricator via libcxx-commits]

alexfh added a comment.

In D144568#4228723 <https://reviews.llvm.org/D144568#4228723>, @alexfh wrote:

> In D144568#4227999 <https://reviews.llvm.org/D144568#4227999>, @philnik wrote:
>
>> In D144568#4227982 <https://reviews.llvm.org/D144568#4227982>, @alexfh wrote:
>>
>>> This commit increases compilation time by ~15% and increases recursive template instantiation depth (thus, requiring adjusting `-ftemplate-depth=2048`) in some of our internal test code. Admittedly, it's using templates in a sort of a compiler-load-test-like way, but this change may affect compilation time for real code as well (it's just not likely to hit the default `-ftemplate-depth=1024` limit and I don't have a good idea of how to find this code with reasonable effort).

I figured out. The test is really a corner case and it's not representative of real code. It artificially creates a chain of 1024 recursive template instantiations and the deepest level started requiring one more template instantiation for std::string after this patch. This is definitely not a problem elsewhere.

As for compilation time regression, it turned out to be much smaller, probably statistically insignificant. The initial difference in compilation time was mostly caused by using different compiler builds. The only consistently reproducible effect on the performance is the aforementioned increase in the number of `NonTypeTemplateParm` and `ConstantExpr` and the corresponding increase in memory usage (negligible relative difference, I would say). If you're interested in looking into this, the reduced test code is here: https://gcc.godbolt.org/z/f5xe4nPfK. It demonstrates the extra template instantiation and the increase in the `ConstantExpr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144568