[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)

Paul Kirth via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 26 18:02:27 PDT 2024


https://github.com/ilovepi requested changes to this pull request.

While this is likely a step in the right direction, I'm a bit skeptical that this patch is entirely correct. 

First, I think we need to understand why we only want to cache/memoize the USR strings from definitions. 

Second, this requires some more significant testing than is currently present for clang-doc. Additionally, we'd need to evaluate such an invasive change on multiple projects, and ensure that the documentation output has not changed due to this patch. Even then, I'd wouldn't be confident that we'd gotten it correct until tests were in place, and we could examine the differences with, and without this patch.

Lastly, I think its worth discussing the memoization/caching strategy used here, and if its appropriate/scalable. Other clang tools use different mechanisms, and it would be good to avoid reinventing the wheel if we can just adopt one of those that also happens to be well tested, such as clangd's indexing and the search/caching mechanisms used in scandeps.

https://github.com/llvm/llvm-project/pull/96809


More information about the cfe-commits mailing list