This adds the references for macros to the SymbolCollector (used for static index).
After this patch:
Num refs increases by 5.7%
Disk usage increases by: 2.5%
Differential D70489
[clangd] Add xref for macro to static index. usaxena95 on Nov 20 2019, 6:22 AM. Authored by
Details
This adds the references for macros to the SymbolCollector (used for static index). After this patch:
Diff Detail
Unit Tests
Event TimelineComment Actions Build result: pass - 60166 tests passed, 0 failed and 730 were skipped.
Comment Actions Build result: pass - 60166 tests passed, 0 failed and 730 were skipped. Log files: console-log.txt, CMakeCache.txt
Comment Actions Build result: pass - 60166 tests passed, 0 failed and 730 were skipped. Log files: console-log.txt, CMakeCache.txt Comment Actions btw, could you measure the increasing size of the index with this patch? You could run ./bin/clangd-indexer -format=binary -executor=all-TUs . > static-index.idx ./bin/dexp static-index.idx # you will see the memory usage.
Comment Actions Addressed comments.
Comment Actions Build result: pass - 60166 tests passed, 0 failed and 730 were skipped. Log files: console-log.txt, CMakeCache.txt Comment Actions Before this patch Loaded Dex from static-index.idx with estimated memory usage 354410880 bytes - number of symbols: 462463 - number of refs: 6361763 - numnber of relations: 20322 Before this patch Loaded Dex from no-macro-xref.idx with estimated memory usage 358596403 bytes - number of symbols: 476056 - number of refs: 6361764 - numnber of relations: 20322 After this patch Loaded Dex from with-macro-xref.idx with estimated memory usage 367648065 bytes - number of symbols: 476069 - number of refs: 6727042 - numnber of relations: 20324 Comment Actions thanks, mostly good, my main concern is that the patch still relies on the CollectMacro and CollectMainFileSymbols option, see my comments below.
Comment Actions Explicitly turn off CollectMacro in test. #Updatin D70489: [clangd] Add xref for macro to static index. Comment Actions Build result: fail - 60403 tests passed, 1 failed and 726 were skipped. failed: LLVM.Bindings/Go/go.test Log files: console-log.txt, CMakeCache.txt Comment Actions Build result: fail - 60403 tests passed, 1 failed and 726 were skipped. failed: LLVM.Bindings/Go/go.test Log files: console-log.txt, CMakeCache.txt
Comment Actions Build result: fail - 60403 tests passed, 1 failed and 726 were skipped. failed: LLVM.Bindings/Go/go.test Log files: console-log.txt, CMakeCache.txt Comment Actions Build result: fail - 60403 tests passed, 1 failed and 726 were skipped. failed: LLVM.Bindings/Go/go.test Log files: console-log.txt, CMakeCache.txt Comment Actions thanks, looks good with a few nits. I think the benchmark data doesn't correct any more with the latest patch, we don't increase number of symbols.
Comment Actions Build result: fail - 60403 tests passed, 1 failed and 726 were skipped. failed: LLVM.Bindings/Go/go.test Log files: console-log.txt, CMakeCache.txt Comment Actions Updated benchmarks: Loaded Dex from static-index.idx with estimated memory usage 354410880 bytes - number of symbols: 462463 - number of refs: 6361763 - numnber of relations: 20322 After this patch Loaded Dex from with-macro-xref.idx with estimated memory usage 363390426 bytes - number of symbols: 462482 - number of refs: 6727051 - numnber of relations: 20325 |
This option is for macro symbols, I'd not use it for collecting macro references. I think the whether to collect macro references is judged by RefFilter and RefsInHeaders.