From 823678ce59a70411ce0f2ec5b7c4b23a7f1bca91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela=20Pastor?= Date: Wed, 11 Oct 2017 07:44:50 -0700 Subject: [PATCH 1/2] Revert reading manifests and statistics without acquiring the lock Unfortunately windows does not allow to replace a file that is opened for reading by another process/thread. Reads and writes need to be serialized by using the locks. This reverts the locking of manifests and statistics to the state before pull request #286. --- clcache.py | 65 +++++++++++++++++++++++++++--------------------------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/clcache.py b/clcache.py index 9d898658..36286ff5 100644 --- a/clcache.py +++ b/clcache.py @@ -1379,7 +1379,7 @@ def printStatistics(cache): called w/ multiple sources : {} called w/ PCH : {}""".strip() - with cache.statistics as stats, cache.configuration as cfg: + with cache.statistics.lock, cache.statistics as stats, cache.configuration as cfg: print(template.format( str(cache), stats.currentCacheSize(), @@ -1666,34 +1666,33 @@ def processDirect(cache, objectFile, compiler, cmdLine, sourceFile): manifestHash = ManifestRepository.getManifestHash(compiler, cmdLine, sourceFile) manifestHit = None manifest = cache.getManifest(manifestHash) - if manifest: - for entryIndex, entry in enumerate(manifest.entries()): - # NOTE: command line options already included in hash for manifest name - try: - includesContentHash = ManifestRepository.getIncludesContentHashForFiles( - [expandBasedirPlaceholder(path) for path in entry.includeFiles]) - - if entry.includesContentHash == includesContentHash: - cachekey = entry.objectHash - assert cachekey is not None - if entryIndex > 0: - # Move manifest entry to the top of the entries in the manifest - with cache.manifestLockFor(manifestHash): - manifest = cache.getManifest(manifestHash) or Manifest() + with cache.manifestLockFor(manifestHash): + if manifest: + for entryIndex, entry in enumerate(manifest.entries()): + # NOTE: command line options already included in hash for manifest name + try: + includesContentHash = ManifestRepository.getIncludesContentHashForFiles( + [expandBasedirPlaceholder(path) for path in entry.includeFiles]) + + if entry.includesContentHash == includesContentHash: + cachekey = entry.objectHash + assert cachekey is not None + if entryIndex > 0: + # Move manifest entry to the top of the entries in the manifest manifest.touchEntry(cachekey) cache.setManifest(manifestHash, manifest) - manifestHit = True - with cache.lockFor(cachekey): - if cache.hasEntry(cachekey): - return processCacheHit(cache, objectFile, cachekey) + manifestHit = True + with cache.lockFor(cachekey): + if cache.hasEntry(cachekey): + return processCacheHit(cache, objectFile, cachekey) - except IncludeNotFoundException: - pass + except IncludeNotFoundException: + pass - unusableManifestMissReason = Statistics.registerHeaderChangedMiss - else: - unusableManifestMissReason = Statistics.registerSourceChangedMiss + unusableManifestMissReason = Statistics.registerHeaderChangedMiss + else: + unusableManifestMissReason = Statistics.registerSourceChangedMiss if manifestHit is None: stripIncludes = False @@ -1706,21 +1705,21 @@ def processDirect(cache, objectFile, compiler, cmdLine, sourceFile): includePaths, compilerOutput = parseIncludesSet(compilerResult[1], sourceFile, stripIncludes) compilerResult = (compilerResult[0], compilerOutput, compilerResult[2]) - if manifestHit is not None: - return ensureArtifactsExist(cache, cachekey, unusableManifestMissReason, - objectFile, compilerResult) + with cache.manifestLockFor(manifestHash): + if manifestHit is not None: + return ensureArtifactsExist(cache, cachekey, unusableManifestMissReason, + objectFile, compilerResult) - entry = createManifestEntry(manifestHash, includePaths) - cachekey = entry.objectHash + entry = createManifestEntry(manifestHash, includePaths) + cachekey = entry.objectHash - def addManifest(): - with cache.manifestLockFor(manifestHash): + def addManifest(): manifest = cache.getManifest(manifestHash) or Manifest() manifest.addEntry(entry) cache.setManifest(manifestHash, manifest) - return ensureArtifactsExist(cache, cachekey, unusableManifestMissReason, - objectFile, compilerResult, addManifest) + return ensureArtifactsExist(cache, cachekey, unusableManifestMissReason, + objectFile, compilerResult, addManifest) def processNoDirect(cache, objectFile, compiler, cmdLine, environment): From 926ddb98df4a7716dd39074693e4d72386ec7ccc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela=20Pastor?= Date: Wed, 11 Oct 2017 08:07:38 -0700 Subject: [PATCH 2/2] read manifest inside lock section --- clcache.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clcache.py b/clcache.py index 36286ff5..bc7a0e77 100644 --- a/clcache.py +++ b/clcache.py @@ -1665,8 +1665,8 @@ def processSingleSource(compiler, cmdLine, sourceFile, objectFile, environment): def processDirect(cache, objectFile, compiler, cmdLine, sourceFile): manifestHash = ManifestRepository.getManifestHash(compiler, cmdLine, sourceFile) manifestHit = None - manifest = cache.getManifest(manifestHash) with cache.manifestLockFor(manifestHash): + manifest = cache.getManifest(manifestHash) if manifest: for entryIndex, entry in enumerate(manifest.entries()): # NOTE: command line options already included in hash for manifest name