================
@@ -5834,13 +5847,27 @@ bool ASTReader::readASTFileControlBlock(
break;
case INPUT_FILE:
bool Overridden = static_cast<bool>(Record[3]);
- const size_t FilenameAsRequestedLength = Record[7] + 1;
- auto ResolvedFilenameAsRequested = ResolveImportedPath(
- PathBuf, Blob.substr(0, FilenameAsRequestedLength), ModuleDir);
- StringRef ExternalFilename = Blob.substr(FilenameAsRequestedLength);
+
+ auto [UnresolvedFilenameAsRequested, UnresolvedFilename] =
+ getUnresolvedInputFilenames(Record, Blob);
+ TemporarilyOwnedStringRef ResolvedFilenameAsRequested =
+ ResolveImportedPath(PathBuf, UnresolvedFilenameAsRequested,
+ ModuleDir);
+ const std::string ResolvedFilenameAsRequestedStr =
+ ResolvedFilenameAsRequested->str();
+
+ std::string ResolvedFilenameStr;
+ if (!UnresolvedFilename.empty()) {
+ SmallString<0> FilenameBuf;
+ FilenameBuf.reserve(256);
+ auto ResolvedFilename =
+ ResolveImportedPath(FilenameBuf, UnresolvedFilename,
ModuleDir);
+ ResolvedFilenameStr = ResolvedFilename->str();
+ }
----------------
jansvoboda11 wrote:
The visitor API makes it sound like it'll pass `Filename` even if it's the same
as `FilenameAsRequested` (which is what I'd expect it to), but here we pass an
empty string. Why?
Also, we're performing unnecessary allocations. This matters with the amount of
files the scanner is processing. The `ResolvedFilenameAsRequested->str()`
allocation is unnecessary when `UnresolvedFilename` is empty (i.e. same as
`UnresolvedFilenameAsRequested`) - you can just reuse the same buffer for both.
When the two paths differ, shuffling data between `std::string` and
`SmallString` is also unnecessary.
I have something like this in mind:
```c++
auto [UnresolvedFilenameAsRequested, UnresolvedFilename] =
getUnresolvedInputFilenames(Record, Blob);
auto TmpPathBufRef = ResolveImportedPath(
PathBuf, UnresolvedFilenameAsRequested, ModuleDir); // no allocation
(amortized)
StringRef FilenameAsRequested = *TmpPathBufRef;
std::string FilenameAsRequestedBuf;
StringRef Filename;
if (UnresolvedFilename.empty())
Filename = FilenameAsRequested;
else {
FilenameAsRequestedBuf = FilenameAsRequested->str(); // allocation (often
elided)
FilenameAsRequested = FilenameAsRequestedBuf;
TmpPathBufRef =
ResolveImportedPath(PathBuf, UnresolvedFilename, ModuleDir); // no
allocation (amortized)
Filename = *TmpPathBufRef;
}
shouldContinue = Listener.visitInputFile(
FilenameAsRequested, Filename, isSystemFile, Overridden,
/*IsExplicitModule=*/false);
```
Alternatively, we can have two "global" `PathBuf` strings so that we can
amortize even the often-elided allocation:
```c++
auto [UnresolvedFilenameAsRequested, UnresolvedFilename] =
getUnresolvedInputFilenames(Record, Blob);
auto FilenameAsRequestedBuf = ResolveImportedPath(
PathBuf1, UnresolvedFilenameAsRequested, ModuleDir); // no allocation
(amortized)
std::optional<TemporarilyOwnedStringRef> FilenameBuf;
StringRef Filename;
if (UnresolvedFilename.empty())
Filename = *FilenameAsRequestedBuf;
else {
FilenameBuf = ResolveImportedPath(
PathBuf2, UnresolvedFilename, ModuleDir); // no allocation (amortized)
Filename = **FilenameBuf;
}
shouldContinue = Listener.visitInputFile(
*FilenameAsRequestedBuf, Filename, isSystemFile, Overridden,
/*IsExplicitModule=*/false);
```
https://github.com/llvm/llvm-project/pull/132237
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
