balazs-benics-sonarsource wrote: > > Should I measure the perf of this change? > > I think so, because the patch is substantially different. For example, the > overly complex symbol tree is now preserved, even if it is apparently not > traversed in its entirety any longer. That makes it not obvious if the > initial performance gains are still there.
Agreed. > Thanks for your patience in this review! I'm happy to see the additional > simplifications coming from just wrapping the over-complicated symbol instead > of storing its parts separately. (I see that it's a bit inelegant to > construct the very thing that we're trying to avoid, but this is the > pragmatic choice...) > I don't expect any performance issues (the "frozen" complex symbols shouldn't > be problematic if they aren't traversed), but performance testing is never > completely useless. I'll do a usual regression test, but not anything as detailed as the evaluation in the PR summary presents. The test case sufficiently demonstrates that this enforcement works as intended. I also had a look at the time trace jsons of the test with the new way of this enforcement and the originally proposed variant. They look identical to the naked eye. I also ran the test with the two versions and they run the same way according to `hyperfine`. Nevertheless, I'll do a usual regression test. https://github.com/llvm/llvm-project/pull/144327 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
