nikic wrote: > To see how effective this would be, a lot of visitors all over the codebase > have been migrated to use the dynamic visitor. The results of this refactor > can be viewed > [here](https://llvm-compile-time-tracker.com/compare.php?from=27e5f505e5eee3da27c1515d6ed95d66fbe543ea&to=8f7d61923b414085d5d1e419cebd46500cef2662&stat=instructions%3Au) > (if you look at the branch in llvm-compile-time-tracker and see that there > are a few commits where _everything_ got magically faster; those are merge > commits, so please ignore those; the link here compares the current state of > this branch to the commit on trunk that it is based on to eliminate any > effects that unrelated changes might have had).
Impressive results! I think the code size improvement is still underestimated by a good bit because the compile-time tracker doesn't build the StaticAnalyzer and ARCMigrate functionality. The build time improvement is also underestimated because unit tests aren't built -- and some of those are very visitor heavy. > However, it is worth noting that using virtual functions for this _does_ seem > to be measurably slower than using CRTP. I have more or less indiscriminately > migrated visitors regardless of how often they are used to see how much of a > slowdown this would incur. Furthermore, I also checked how often each visitor > is being instantiated when compiling Clang itself. The results of that are > shown below. The compile-time impact here looks pretty low -- clang fairly routinely accepts compile-time regressions of that magnitude. Something I noticed looking at the per-file stats (https://llvm-compile-time-tracker.com/compare.php?from=27e5f505e5eee3da27c1515d6ed95d66fbe543ea&to=8f7d61923b414085d5d1e419cebd46500cef2662&stat=instructions%3Au&details=on) is that the impact is primarily on very small objects, and linking is also impacted, which is not something we would usually expect from a clang frontend change. My suspicion is thus that this actually marginally regresses binary loading time. An obvious guess would be that this introduces many large vtables, and accordingly also many dynamic relocations that need to be resolved by the dynamic linker. (This is the problem that `-fexperimental-relative-c++-abi-vtables` solves, but well, experimental.) So if there is some way to reduce the number of virtual methods, that would be good, but otherwise this just seems like the price of doing business... ----- In terms of landing this, I'd split this up in at least three parts, which is 1. the implementation (which is the part that will need detailed review), 2. converting unit tests (that should be a no-brainer) and 3. migrating production visitors, possibly further split down (e.g. something like ARCMigrate is self-contained). https://github.com/llvm/llvm-project/pull/105195 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
