Merge pull request #1057 from nextcloud/cache-negative

perf: cache negative results in LockSerivice::getLockForNodeIds

Author: benjaminfrueh

lib/Service/LockService.php

@@ -55,6 +55,7 @@ class LockService {
 	private array $locks = [];
 	private bool $lockRetrieved = false;
 	private array $lockCache = [];
+	private array $remoteLockCache = [];
 	private ?array $directEditors = null;
 	private bool $allowUserOverride = false;
 
@@ -90,18 +91,26 @@ public function __construct(
 	 * @return FileLock|bool
 	 */
 	public function getLockForNodeId(int $nodeId, ?Node $node = null) {
-		if (array_key_exists($nodeId, $this->lockCache) && $this->lockCache[$nodeId] !== null) {
+		if (array_key_exists($nodeId, $this->lockCache) && $this->lockCache[$nodeId] !== false) {
 			return $this->lockCache[$nodeId];
 		}
 
-		try {
-			$this->lockCache[$nodeId] = $this->getLockFromFileId($nodeId);
-		} catch (LockNotFoundException) {
-			$remoteLock = $this->getRemoteLockFromDav($nodeId, $node);
-			$this->lockCache[$nodeId] = $remoteLock ?: false;
+		if (array_key_exists($nodeId, $this->remoteLockCache)) {
+			return $this->remoteLockCache[$nodeId];
+		}
+
+		if (!array_key_exists($nodeId, $this->lockCache)) {
+			try {
+				$this->lockCache[$nodeId] = $this->getLockFromFileId($nodeId);
+				return $this->lockCache[$nodeId];
+			} catch (LockNotFoundException) {
+				$this->lockCache[$nodeId] = false;
+			}
 		}
 
-		return $this->lockCache[$nodeId];
+		$remoteLock = $this->getRemoteLockFromDav($nodeId, $node);
+		$this->remoteLockCache[$nodeId] = $remoteLock ?: false;
+		return $this->remoteLockCache[$nodeId];
 	}
 
 	/**
@@ -113,8 +122,10 @@ public function getLockForNodeIds(array $nodeIds): array {
 		$locks = [];
 		$locksToRequest = [];
 		foreach ($nodeIds as $nodeId) {
-			if (array_key_exists($nodeId, $this->lockCache) && $this->lockCache[$nodeId] !== null) {
+			if (array_key_exists($nodeId, $this->lockCache) && $this->lockCache[$nodeId] instanceof FileLock) {
 				$locks[$nodeId] = $this->lockCache[$nodeId];
+			} elseif (array_key_exists($nodeId, $this->remoteLockCache)) {
+				$locks[$nodeId] = $this->remoteLockCache[$nodeId];
 			} else {
 				$locksToRequest[] = $nodeId;
 			}
@@ -123,6 +134,12 @@ public function getLockForNodeIds(array $nodeIds): array {
 			return $locks;
 		}
 
+		// pre-fill the cache with negative hits for all requested ids
+		// so if no lock is found for the file we store the negative hit
+		foreach ($locksToRequest as $fileId) {
+			$this->lockCache[$fileId] = false;
+		}
+
 		$newLocks = [];
 		while ($fileIds = array_splice($locksToRequest, 0, 1000)) {
 			$newLocks[] = $this->locksRequest->getFromFileIds($fileIds);