Change Details

This patch makes template instantiations be already performed in the PCH instead of it being done in every single file that uses the PCH. I can see 20-30% build time saved with the few tests I've tried. The delaying of template instantiations until the PCH is used comes from 7f76d11dcc9196e1fc9d1308da9ed2330a6b06c2 , which doesn't really give any useful reasoning for it, except for extending a unittest, which however now passes even with the instantiation moved back into the PCH. The only way this breaks things that I've managed to find is if a .cpp file using the PCH adds another template specialization that's not mentioned in the PCH, which is why this needs to be enabled by a flag, but PCHs often require tweaking, and in this case e.g. adding a forward declaration for the specialization seems to be very well worth it, in the rare case it happens. I've built a complete debug build of LibreOffice with the patch and everything seems to work fine. Even all Clang tests pass fine with this flag forced, except for ~22 tests which fail merely because this change reorders things differently than the tests expect, without any actual change, and one OpenMP test, for which I had a suggested fix in an earlier version of this patch, but since that version went without a reaction, as long as this change needs to be opted-in, I'll leave that to whomever cares about OpenMP.

This patch makes template instantiations be already performed in the PCH instead of it being done in every single file that uses the PCH (but every single file will still do it as well in order to handle its own instantiations). I can see 20-30% build time saved with the few tests I've tried. The delaying of template instantiations until the PCH is used comes from 7f76d11dcc9196e1fc9d1308da9ed2330a6b06c2 , which doesn't really give any useful reasoning for it, except for extending a unittest, which however now passes even with the instantiation moved back into the PCH. The only way this breaks things that I've managed to find is if a .cpp file using the PCH adds another template specialization that's not mentioned in the PCH, which is why this needs to be enabled by a flag, but PCHs often require tweaking, and in this case e.g. adding a forward declaration for the specialization seems to be very well worth it, in the rare case it happens. I've built a complete debug build of LibreOffice with the patch and everything seems to work fine. Even all Clang tests pass fine with this flag forced, except for ~22 tests which fail merely because this change reorders things differently than the tests expect, without any actual change, and one OpenMP test, for which I a suggested fix is at https://reviews.llvm.org/D72759 . Since this happens only when explicitly opting in, I do not consider this to be a blocker.

This patch makes template instantiations be already performed in the PCH instead of it being done in every single file that uses the PCH (but every single file will still do it as well in order to handle its own instantiations). I can see 20-30% build time saved with the few tests I've tried. The delaying of template instantiations until the PCH is used comes from 7f76d11dcc9196e1fc9d1308da9ed2330a6b06c2 , which doesn't really give any useful reasoning for it, except for extending a unittest, which however now passes even with the instantiation moved back into the PCH. The only way this breaks things that I've managed to find is if a .cpp file using the PCH adds another template specialization that's not mentioned in the PCH, which is why this needs to be enabled by a flag, but PCHs often require tweaking, and in this case e.g. adding a forward declaration for the specialization seems to be very well worth it, in the rare case it happens. I've built a complete debug build of LibreOffice with the patch and everything seems to work fine. Even all Clang tests pass fine with this flag forced, except for ~22 tests which fail merely because this change reorders things differently than the tests expect, without any actual change, and one OpenMP test, for which I had a suggested fix in an earlier version of this patch, but since that version went without a reaction,is at https://reviews.llvm.org/D72759 . as long asSince this change needs to behappens only when explicitly opted-ining in, I'll leave that to whomever cares about OpenMP. I do not consider this to be a blocker.