SourceManager: Fix an SLocEntry memory regression introduced with FileEntryRef

4dc5573acc0d2e7c59d8bac2543eb25cb4b32984 added `FileEntryRef` in order to
help enable sharing of a `FileManager` betwee

SourceManager: Fix an SLocEntry memory regression introduced with FileEntryRef

4dc5573acc0d2e7c59d8bac2543eb25cb4b32984 added `FileEntryRef` in order to
help enable sharing of a `FileManager` between `CompilerInstance`s.

It also added a `StringRef` with the filename on `FileInfo`. This
doubled `sizeof(FileInfo)`, bloating `sizeof(SLocEntry)`, of which we
have one for each (loaded and unloaded) file and macro expansion. This
causes a memory regression in modules builds.

Move the filename down into the `ContentCache`, which is a side data
structure for `FileInfo` that does not impact `sizeof(SLocEntry)`. Once
`FileEntryRef` is used for `ContentCache::OrigEntry` this can go away.

Differential Revision: https://reviews.llvm.org/D89580
Radar-Id: rdar://59908826

show more ...

SourceManager: Return non-const references in getOrCreateContentCache and related, NFC

Update a few APIs to return non-const references instead of pointers,
and remove associated `const_cast`s and n

SourceManager: Return non-const references in getOrCreateContentCache and related, NFC

Update a few APIs to return non-const references instead of pointers,
and remove associated `const_cast`s and non-null assertions.

Differential Revision: https://reviews.llvm.org/D90067

show more ...

SourceManager: Simplify by inlining what remains of ComputeLineNumbers, NFC

Use `LineOffsetMapping:get` directly and remove/inline the helper
`ComputeLineNumbers`, simplifying the callers.

Differen

SourceManager: Simplify by inlining what remains of ComputeLineNumbers, NFC

Use `LineOffsetMapping:get` directly and remove/inline the helper
`ComputeLineNumbers`, simplifying the callers.

Differential Revision: https://reviews.llvm.org/D89922

show more ...

SourceManager: Make LastLineNoContentCache and ContentCache::SourceLineCache mutable, NFC

Avoid some noisy `const_cast`s by making `ContentCache::SourceLineCache`
and `SourceManager::LastLineNoConte

SourceManager: Make LastLineNoContentCache and ContentCache::SourceLineCache mutable, NFC

Avoid some noisy `const_cast`s by making `ContentCache::SourceLineCache`
and `SourceManager::LastLineNoContentCache` both mutable.

Differential Revision: https://reviews.llvm.org/D89914

show more ...

SourceManager: Encapsulate line number mapping into SrcMgr::LineOffsetMapping

Put the guts of `ComputeLineNumbers` into `LineOffsetMapping::get` and
`LineOffsetMapping::LineOffsetMapping`. As a dri

SourceManager: Encapsulate line number mapping into SrcMgr::LineOffsetMapping

Put the guts of `ComputeLineNumbers` into `LineOffsetMapping::get` and
`LineOffsetMapping::LineOffsetMapping`. As a drive-by, store the number
of lines directly in the bump-ptr-allocated array.

Differential Revision: https://reviews.llvm.org/D89913

show more ...

SourceManager: Clarify that FileInfo always has a ContentCache, NFC

It turns out that `FileInfo` *always* has a ContentCache. Clarify that
in the code:
- Update the private version of `SourceManager

SourceManager: Clarify that FileInfo always has a ContentCache, NFC

It turns out that `FileInfo` *always* has a ContentCache. Clarify that
in the code:
- Update the private version of `SourceManager::createFileID` to take a
`ContentCache&` instead of `ContentCache*`, and rename it to
`createFileIDImpl` for clarity.
- Change `FileInfo::getContentCache` to return a reference.

Differential Revision: https://reviews.llvm.org/D89554

show more ...

SourceManager: Remove a redundant nullptr check in getNonBuiltinFilenameForID, NFC

SourceManager: getFileEntryRefForID => getNonBuiltinFilenameForID, NFC

`SourceManager::getFileEntryRefForID`'s remaining callers just want the
filename component, which is coming from the `FileInfo`

SourceManager: getFileEntryRefForID => getNonBuiltinFilenameForID, NFC

`SourceManager::getFileEntryRefForID`'s remaining callers just want the
filename component, which is coming from the `FileInfo`. Replace the API
with `getNonBuiltinFilenameForID`, which also removes another use of
`FileEntryRef::FileEntryRef` outside of `FileManager`.

Both callers are collecting file dependencies, and one of them relied on
this API to filter out built-ins (as exposed by
clang/test/ClangScanDeps/modules-full.cpp). It seems nice to continue
providing that service.

Differential Revision: https://reviews.llvm.org/D89508

show more ...

SourceManager: Change SourceManager::isMainFile to take a FileEntry, NFC

`SourceManager::isMainFile` does not use the filename, so it doesn't
need the full `FileEntryRef`; in fact, it's misleading t

SourceManager: Change SourceManager::isMainFile to take a FileEntry, NFC

`SourceManager::isMainFile` does not use the filename, so it doesn't
need the full `FileEntryRef`; in fact, it's misleading to take the name
because that makes it look relevant. Simplify the API, and in the
process remove some calls to `FileEntryRef::FileEntryRef` in the unit
tests (which were blocking making that private to `SourceManager`).

Differential Revision: https://reviews.llvm.org/D89507

show more ...

SourceManager: Factor out helpers for common SLocEntry lookup pattern, NFC

Add helpers `getSLocEntryOrNull`, which handles the `Invalid` logic
around `getSLocEntry`, and `getSLocEntryForFile`, which

SourceManager: Factor out helpers for common SLocEntry lookup pattern, NFC

Add helpers `getSLocEntryOrNull`, which handles the `Invalid` logic
around `getSLocEntry`, and `getSLocEntryForFile`, which also checks for
`SLocEntry::isFile`, and use them to reduce repeated code.

Differential Revision: https://reviews.llvm.org/D89503

show more ...

[SourceManager] Avoid copying SLocEntry in computeMacroArgsCache

Follow-up to e7870223d8b5

Differential Revision: https://reviews.llvm.org/D86230

[SourceManager] Skip module maps when searching files for macro arguments

Differential Revision: https://reviews.llvm.org/D86230

clang/Basic: Remove ContentCache::getRawBuffer, NFC

Replace `ContentCache::getRawBuffer` with `getBufferDataIfLoaded` and
`getBufferIfLoaded`, excising another accessor for the underlying
`MemoryBuf

clang/Basic: Remove ContentCache::getRawBuffer, NFC

Replace `ContentCache::getRawBuffer` with `getBufferDataIfLoaded` and
`getBufferIfLoaded`, excising another accessor for the underlying
`MemoryBuffer*` in favour of `StringRef` and `MemoryBufferRef`.

Differential Revision: https://reviews.llvm.org/D89445

show more ...

SourceManager: Simplify early returns in ContentCache::getBufferOrNone, NFC

As suggested in the review for https://reviews.llvm.org/D89430, simplify
the logic for marking the buffer as invalid in th

SourceManager: Simplify early returns in ContentCache::getBufferOrNone, NFC

As suggested in the review for https://reviews.llvm.org/D89430, simplify
the logic for marking the buffer as invalid in the early return paths.

Differential Revision: https://reviews.llvm.org/D89722

show more ...

ContentCache: Simplify by always owning the MemoryBuffer

This changes `ContentCache::Buffer` to use
`std::unique_ptr<MemoryBuffer>` instead of the `PointerIntPair`. It
drops the (mostly unused) `DoN

ContentCache: Simplify by always owning the MemoryBuffer

This changes `ContentCache::Buffer` to use
`std::unique_ptr<MemoryBuffer>` instead of the `PointerIntPair`. It
drops the (mostly unused) `DoNotFree` bit, instead creating a (new)
non-owning `MemoryBuffer` instance when passed a `MemoryBufferRef`.

Differential Revision: https://reviews.llvm.org/D67030

show more ...

clang/Basic: ContentCache::InvalidFlag => ContentCache::IsBufferInvalid, NFC

Move a flag out of the `MemoryBuffer*` to unblock changing it to a
`unique_ptr`. There are plenty of bits available in th

clang/Basic: ContentCache::InvalidFlag => ContentCache::IsBufferInvalid, NFC

Move a flag out of the `MemoryBuffer*` to unblock changing it to a
`unique_ptr`. There are plenty of bits available in the bitfield below.

Differential Revision: https://reviews.llvm.org/D89431

show more ...

clang/Basic: Remove SourceManager::getBufferPointer, NFC

Inline `Source::getBufferPointer` into its only remaining caller,
`getBufferOrNone`. No functionality change.

Differential Revision: https:/

clang/Basic: Remove SourceManager::getBufferPointer, NFC

Inline `Source::getBufferPointer` into its only remaining caller,
`getBufferOrNone`. No functionality change.

Differential Revision: https://reviews.llvm.org/D89430

show more ...

clang/Basic: Replace SourceManager::getMemoryBufferForFile, NFC

Replace `SourceManager::getMemoryBufferForFile`, which returned a
dereferenceable `MemoryBuffer*` and had a `bool*Invalid` out paramet

clang/Basic: Replace SourceManager::getMemoryBufferForFile, NFC

Replace `SourceManager::getMemoryBufferForFile`, which returned a
dereferenceable `MemoryBuffer*` and had a `bool*Invalid` out parameter,
with `getMemoryBufferForFileOrNone` (returning
`Optional<MemoryBufferRef>`) and `getMemoryBufferForFileOrFake`
(returning `MemoryBufferRef`).

Differential Revision: https://reviews.llvm.org/D89429

show more ...

clang/Frontend: Use MemoryBufferRef in FrontendInputFile (and remove SourceManager::getBuffer)

In order to drop the final callers to `SourceManager::getBuffer`, change
`FrontendInputFile` to use `Op

clang/Frontend: Use MemoryBufferRef in FrontendInputFile (and remove SourceManager::getBuffer)

In order to drop the final callers to `SourceManager::getBuffer`, change
`FrontendInputFile` to use `Optional<MemoryBufferRef>`. Also updated
the "unowned" version of `SourceManager::createFileID` to take a
`MemoryBufferRef` (it now calls `MemoryBuffer::getMemBuffer`, which
creates a `MemoryBuffer` that does not own the buffer data).

Differential Revision: https://reviews.llvm.org/D89427

show more ...

clang/Basic: Stop using SourceManager::getBuffer, NFC

Update clang/lib/Basic to stop relying on a `MemoryBuffer*`, using the
`MemoryBufferRef` from `getBufferOrNone` or `getBufferOrFake` instead of

clang/Basic: Stop using SourceManager::getBuffer, NFC

Update clang/lib/Basic to stop relying on a `MemoryBuffer*`, using the
`MemoryBufferRef` from `getBufferOrNone` or `getBufferOrFake` instead of
`getBuffer`.

Differential Revision: https://reviews.llvm.org/D89394

show more ...

clang/Basic: Replace ContentCache::getBuffer with Optional semantics

Remove `ContentCache::getBuffer`, which always returned a
dereferenceable `MemoryBuffer*` and had a `bool*Invalid` out parameter,

clang/Basic: Replace ContentCache::getBuffer with Optional semantics

Remove `ContentCache::getBuffer`, which always returned a
dereferenceable `MemoryBuffer*` and had a `bool*Invalid` out parameter,
and replace it with:

- `ContentCache::getBufferOrNone`, which returns
`Optional<MemoryBufferRef>`. This is the new API that consumers should
use. Later it could be renamed to `getBuffer`, but intentionally using
a different name to root out any unexpected callers.
- `ContentCache::getBufferPointer`, which returns `MemoryBuffer*` with
"optional" semantics. This is `private` to avoid growing callers and
`SourceManager` has temporarily been made a `friend` to access it.
Later paches will update the transitive callers to not need a raw
pointer, and eventually this will be deleted.

No functionality change intended here.

Differential Revision: https://reviews.llvm.org/D89348

show more ...

[SourceManager] Explicitly check for potential iterator underflow

Differential Revision: https://reviews.llvm.org/D86231

Reland "[clang][SourceManager] cache Macro Expansions""

This reverts commit 33d63f02ce408d181e13089ee5a667fb2e1cdc78.

Differential Revision: https://reviews.llvm.org/D80681

Revert "[clang][SourceManager] cache Macro Expansions"

This reverts commit dffc1420451f674731cb36799c8ae084104ff0b5.

Missed a hunk (D82690).

[clang][SourceManager] cache Macro Expansions

A seemingly innocuous Linux kernel change [0] seemingly blew up our
compile times by over 3x, as reported by @nathanchance in [1].

The code in question

[clang][SourceManager] cache Macro Expansions

A seemingly innocuous Linux kernel change [0] seemingly blew up our
compile times by over 3x, as reported by @nathanchance in [1].

The code in question uses a doubly nested macro containing GNU C
statement expressions that are then passed to typeof(), which is then
used in a very important macro for atomic variable access throughout
most of the kernel. The inner most macro, is passed a GNU C statement
expression. In this case, we have macro arguments that are GNU C
statement expressions, which can contain a significant number of tokens.
The upstream kernel patch caused significant build time regressions for
both Clang and GCC. Since then, some of the nesting has been removed via
@melver, which helps gain back most of the lost compilation time. [2]

Profiles collected [3] from compilations of the slowest TU for us in the
kernel show:
* 51.4% time spent in clang::TokenLexer::updateLocForMacroArgTokens
* 48.7% time spent in clang::SourceManager::getFileIDLocal
* 35.5% time spent in clang::SourceManager::isOffsetInFileID
(mostly calls from the former through to the latter).

So it seems we have a pathological case for which properly tracking the
SourceLocation of macro arguments is significantly harming build
performance. This stands out in referenced flame graph.

In fact, this case was identified previously as being problematic in
commit 3339c568c4 ("[Lex] Speed up updateConsecutiveMacroArgTokens (NFC)")

Looking at the above call chain, there's 3 things we can do to speed up
this case.

1. TokenLexer::updateConsecutiveMacroArgTokens() calls
SourceManager::isWrittenInSameFile() which calls
SourceManager::getFileID(), which is both very hot and very expensive
to call. SourceManger has a one entry cache, member LastFileIDLookup.
If that isn't the FileID for a give source location offset, we fall
back to a linear probe, and then to a binary search for the FileID.
These fallbacks update the one entry cache, but noticeably they do
not for the case of macro expansions!

For the slowest TU to compile in the Linux kernel, it seems that we
miss about 78.67% of the 68 million queries we make to getFileIDLocal
that we could have had cache hits for, had we saved the macro
expansion source location's FileID in the one entry cache. [4]

I tried adding a separate cache item for macro expansions, and to
check that before the linear then binary search fallbacks, but did
not find it faster than simply allowing macro expansions into the one
item cache. This alone nets us back a lot of the performance loss.

That said, this is a modification of caching logic, which is playing
with a double edged sword. While it significantly improves the
pathological case, its hard to say that there's not an equal but
opposite pathological case that isn't regressed by this change.
Though non-pathological cases of builds of the Linux kernel before
[0] are only slightly improved (<1%) and builds of LLVM itself don't
change due to this patch.

Should future travelers find this change to significantly harm their
build times, I encourage them to feel empowered to revert this
change.

2. SourceManager::getFileIDLocal has a FIXME hinting that the call to
SourceManager::isOffsetInFileID could be made much faster since
isOffsetInFileID is generic in the sense that it tries to handle the
more generic case of "local" (as opposed to "loaded") files, though
the caller has already determined the file to be local. This patch
implements a new method that specialized for use when the caller
already knows the file is local, then use that in
TokenLexer::updateLocForMacroArgTokens. This should be less
controversial than 1, and is likely an across the board win. It's
much less significant for the pathological case, but still a
measurable win once we have fallen to the final case of binary
search. D82497

3. A bunch of methods in SourceManager take a default argument.
SourceManager::getLocalSLocEntry doesn't do anything with this
argument, yet many callers of getLocalSLocEntry setup, pass, then
check this argument. This is wasted work. D82498

With this patch applied, the above profile [5] for the same pathological
input looks like:
* 25.1% time spent in clang::TokenLexer::updateLocForMacroArgTokens
* 17.2% time spent in clang::SourceManager::getFileIDLocal
and clang::SourceManager::isOffsetInFileID is no longer called, and thus
falls out of the profile.

There may be further improvements to the general problem of "what
interval contains one number out of millions" than the current use of a
one item cache, followed by linear probing, followed by binary
searching. We might even be able to do something smarter in
TokenLexer::updateLocForMacroArgTokens.

[0] https://github.com/ClangBuiltLinux/linux/commit/cdd28ad2d8110099e43527e96d059c5639809680
[1] https://github.com/ClangBuiltLinux/linux/issues/1032
[2] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=locking/kcsan&id=a5dead405f6be1fb80555bdcb77c406bf133fdc8
[3] https://github.com/ClangBuiltLinux/linux/issues/1032#issuecomment-633712667
[4] https://github.com/ClangBuiltLinux/linux/issues/1032#issuecomment-633741923
[5] https://github.com/ClangBuiltLinux/linux/issues/1032#issuecomment-634932736

Reviewed By: kadircet

Differential Revision: https://reviews.llvm.org/D80681

show more ...