[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

MaskRay added a comment. > Speedup: Linux kernel: getFileIDLocal overhead is reduced to 1.66% (~30% > saving) What's the fraction related to the whole parsing time? > The sad bit is thatit increases SourceManager memory usage, however it is a > small fraction (< 10%) of clang AST memory

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

hokein added a comment. In D135440#3847720 , @nickdesaulniers wrote: > Is it worth it and possible to fully decompose `LocalSLocEntryTable` into > arrays of its constituent parts, and only construct a `SLocEntry` when > necessary? Fully decomposing

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

hokein updated this revision to Diff 466729. hokein marked an inline comment as done. hokein added a comment. add assert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135440/new/ https://reviews.llvm.org/D135440 Files:

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Is it worth it and possible to fully decompose `LocalSLocEntryTable` into arrays of its constituent parts, and only construct a `SLocEntry` when necessary? I'm ok with the

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

hokein added a comment. More data on clang memory usage (measured with the `clang -cc1 -fsyntax-only -print-stats`, AST size = ASTContext + Decl + Stmt/Expr; Total = AST size + SourceManager size) The memory increase for C++ is negligible (< 1%), but for C, we have ~5% memory increase. |

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

hokein added a comment. >> SemaExpr.cpp: 149529494 -> 149502874 > > Are these numbers backwards? Why would memory usage go down? Oops, you're right, I made a mistake, correct it in the description. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

hokein updated this revision to Diff 466595. hokein added a comment. print memory usage in SourceManager::PrintStats. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135440/new/ https://reviews.llvm.org/D135440 Files:

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

sammccall added a comment. It'd be great to have some numbers for overall memory increase based on clang rather than clangd, that's a more critical case and has a different memory pattern. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135440/new/

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

sammccall added a reviewer: aaron.ballman. sammccall added a subscriber: aaron.ballman. sammccall added a comment. This looks good to me, I think we should ask @aaron.ballman about the memory increase. If we wanted to eliminate SLocEntry::Offset then we could add

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/SourceManager.h:697 + /// FileID lookup. + SmallVector LocalLocOffsetTable; nickdesaulniers wrote: > `LocalSLocEntryTable` is a vec of `SrcMgr::SLocEntry`. `SrcMgr::SLocEntry`'s >

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/SourceManager.h:696 + /// An in-parallel SlocEntry offset table, merely used for speeding up the + /// FileID lookup. + SmallVector LocalLocOffsetTable; Add to this comment that the

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

hokein created this revision. hokein added reviewers: sammccall, nickdesaulniers. Herald added a subscriber: kadircet. Herald added a project: All. hokein requested review of this revision. Herald added a subscriber: ilya-biryukov. Herald added a project: clang. This patch introduces a separate