[slydiman]

This patch fixes swiftlang/llvm-project / issues / 12605 (avoid ref for now).
We have reproduced the issue on x86_64 too running 2000 processes simultaneously.
It is hard to create a stable test for this patch.

[andreil99]

Tanks for the patch! Could you add a comment with the rationale of the fix, please? And maybe a reference to a KB article or whatever Microsoft doc describing the specifics?

[slydiman]

I couldn't find a KB article or an explicit mention of this in the Microsoft documentation.
The most likely scenario is the following:
Intensive, chaotic writing of small blocks to sparse files via MMF leads to an exponential increase in the number of entries in the NTFS MFT. The maximum file fragmentation limit (ERROR_FILE_SYSTEM_LIMITATION) is triggered by the NTFS file system itself when attempting to allocate a new cluster or attribute. If this occurs while the Modified Page Writer is active, the NTFS driver returns an I/O error to the memory subsystem; however, since the application thread has already exited UnmapViewOfFile, the data is discarded from the cache.

I think that avoiding sparse files will reduce fragmentation and improve the performance.

[tagolog]

I tried this patch with our standalone repro, and it fixes the symptom we saw in CI.
However, it is still not enough: cc1 part still loses writes.

The driver passes -disable-free to every cc1 job (Clang.cpp L5671-L5675).

At the end, cc1_main buries the CompilerInstance instead of destroying it (cc1_main.cpp L373-L377).

BuryPointer only does unique_ptr::release() and stores the raw pointer in a static array (BuryPointer.cpp L15-L28), so no destructor of the instance is ever called. That means neither MappedFileRegionArena::destroyImpl() nor mapped_file_region::unmapImpl() runs, and the FlushViewOfFile never happens.

In the repro, this patch brings the depscan failures to 0, but cc1 writes are still lost. Only when cc1 also closes the CAS before exiting do we get 0 lost writes. Since cc1 stores the compile result, I'd expect the issues later, on cache hits.

Upstream already destroys one object explicitly under -disable-free (llvm/llvm-project cc1_main.cpp L331-L337):

  if (Clang->getFrontendOpts().DisableFree) {
    // DiagnosticConsumer must be always destroyed.
    Clang->getDiagnosticClient().~DiagnosticConsumer();
    llvm::BuryPointer(std::move(Clang));

The CAS needs the same: release the ObjectStore/ActionCache references held by CompilerInstance and CompileJobCache before BuryPointer, so destroyImpl() runs and flushes.

[slydiman]

@tagolog
I don't agree. This patch is being proposed for mainline.
There is no cc1_main() in mainline for now at all.
Let's see how CAS is initialized and stored in cc1_main()...
cc1_main() contains CompileJobCache JobCache;

CompileJobCache::initialize() calls CompilerInstance::getOrCreateCASDatabases() -> getCASOpts().createDatabases() -> CASConfiguration::createDatabases()

CompileJobCache stores

  std::shared_ptr<llvm::cas::ObjectStore> CAS;
  std::shared_ptr<llvm::cas::ActionCache> Cache;
and
  std::unique_ptr<CachingOutputs> CacheBackend;

Note it is shared_ptr, not unique_ptr. I think it is necessary to keep CAS and Cache inside CacheBackend too.

The variable Clang (CompilerInstance) inside cc1_main() does not store CAS ObjectStore or ActionCache.
The variable JobCache must be destroyed at the end of cc1_main() with CAS and Cache objects and -disable-free is not affected.

[slydiman]

CAS and Cache objects are not destroyed at the end of cc1_main() because the shared_ptr ref counter is still 1.
I'm trying to find where is the extra storage.

[slydiman]

Note cc1_main() destroys JobCache and CAS & Cache objects in case of hit and early exit here

  // Check for a cache hit.
  if (std::optional<int> Status = JobCache.tryReplayCachedResult(*Clang))
    return *Status; // FIXME: Should write out timers before exiting!

[slydiman]

Ok, Clang (CompilerInstance) stores the copy

  /// The CAS, if any.
  std::shared_ptr<llvm::cas::ObjectStore> CAS;
  /// The ActionCache, if any.
  std::shared_ptr<llvm::cas::ActionCache> ActionCache;

In the patch for swiftlang we need to release them at the end of cc1_main() in case of DisableFree.
Try to call Clang->setCASDatabases(nullptr, nullptr);
before llvm::BuryPointer(std::move(Clang));

[tagolog]

Agreed, let's split it. I vote for you to proceed with the mainline PR for the FlushViewOfFile() part, and I'll fix the remaining part on the SwiftLang side.