Improve filesystem repo caching

CHANGELOG.md

@@ -10,9 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - #1117: Add `sync` command for incremental loading of changed files in dev-mode modules. Detects modified files since last sync using SHA-1 hash and recompiles only what is stale. Supports `-delete` for processing removed files and `-test` for running changed test-phase unit tests.
 - #106: A module can now specify `<SystemRequirements SYSNamespace="true"/>` to prevent installation in non-%SYS namespaces.
+- #536: Improve filesystem repository cache through performance improvements, smart auto-cache rebuilding on install, and new `-rebuild-cache` flag for `repo` command to manually rebuild the entire cache.
 
 ### Changed
 - Minimum supported Python version is now 3.9
+- #536: An explicit `-depth` on a filesystem repository is no longer overwritten during the initial scan, so a module added at a level within the configured depth is still discovered.
 
 ### Fixed
 - Performance: Studio project creation on package load in dev mode is now 80% faster.

src/cls/IPM/General/Sync/Pipeline.cls

@@ -36,7 +36,7 @@ ClassMethod Run(
         }
 
         // Step 1: Check if module.xml changed; reload manifest if so
-        set moduleXmlRelPath = ##class(%IPM.Storage.FileHash).NormalizePath("module.xml")
+        set moduleXmlRelPath = ##class(%IPM.Utils.File).NormalizePath("module.xml")
         set moduleXmlPath = root _ "module.xml"
         set moduleXmlChanged = ..SyncCheckModuleXml(.module, moduleXmlPath, moduleXmlRelPath)
 
@@ -305,7 +305,7 @@ ClassMethod SyncBuildReverseIndex(module As %IPM.Storage.Module, orderedResource
                 set relPath = resource.Processor.OnItemRelativePath(childName)
             }
             if relPath '= "" {
-                set normalizedRelPath = ##class(%IPM.Storage.FileHash).NormalizePath(relPath)
+                set normalizedRelPath = ##class(%IPM.Utils.File).NormalizePath(relPath)
                 set reverseIndex(normalizedRelPath) = resource.Name
                 set reverseIndex(normalizedRelPath, "Processor") = resource.Processor
                 set reverseIndex(normalizedRelPath, "Resource") = resource
@@ -320,7 +320,7 @@ ClassMethod SyncBuildReverseIndex(module As %IPM.Storage.Module, orderedResource
         // directory. Prefix-scan allFiles to map every file under that dir to this resource.
         set syncDir = resource.Processor.GetSyncDirectory()
         if syncDir '= "" {
-            set prefix = ##class(%IPM.Storage.FileHash).NormalizePath(syncDir _ "/")
+            set prefix = ##class(%IPM.Utils.File).NormalizePath(syncDir _ "/")
             set prefixLen = $length(prefix)
             set dirRelPath = prefix
             for {

src/cls/IPM/General/TempLocalRepoManager.cls

@@ -27,7 +27,7 @@ Method Create(useFirst As %Boolean) [ Internal, Private ]
     // Make sure this is the first/last repo to be found by SQL query in %IPM.Repo.Manager:SearchRepositoriesForModule
     set ..Repo.OverriddenSortOrder = $select(useFirst:-1000 ,1:1000)
 
-    $$$ThrowOnError(..Repo.BuildCache(1,1,1))
+    $$$ThrowOnError(..Repo.BuildCache(1,1))
 }
 
 ClassMethod SkipCreate(location As %String) As %Boolean [ Internal ]

src/cls/IPM/Lifecycle/Module.cls

@@ -41,6 +41,8 @@ Method %Clean(ByRef pParams) As %Status
                 kill ^IPM.Repo.DefinitionD
                 kill ^IPM.Repo.DefinitionI
                 kill ^IPM.Repo.Filesystem.CacheD
+                kill ^IPM.Repo.Filesystem.CacheI
+                kill ^IPM.Repo.Filesystem.CacheS
                 kill ^IPM.General.SettingsD
                 kill ^IPM.UpdateStep.AnyMemberD
                 kill ^IPM.UpdateStep.PrimaryOnlyD

src/cls/IPM/Main.cls

@@ -2193,6 +2193,29 @@ ClassMethod Repository(ByRef pCommandInfo) [ Internal ]
         set tType = "%IPM.Repo.Remote.Definition"
         $$$ThrowOnError($classmethod(tType,"Configure",1,.tModifiers,.tData))
         do ..Shell("repo -list")
+    } elseif $$$HasModifier(pCommandInfo,"rebuild-cache") {
+        // Validate repository exists and is filesystem type
+        set repoName = $$$GetModifier(pCommandInfo,"name")
+        if (repoName = "") {
+            $$$ThrowStatus($$$ERROR($$$GeneralError,"-rebuild-cache requires -name <repo>"))
+        }
+        set serverDef = ##class(%IPM.Repo.Definition).ServerDefinitionKeyOpen(repoName,,.sc)
+        $$$ThrowOnError(sc)
+
+        if 'serverDef.%IsA("%IPM.Repo.Filesystem.Definition") {
+            $$$ThrowStatus($$$ERROR($$$GeneralError,"Cache rebuild only supported for filesystem repositories"))
+        }
+
+        // Rebuild cache with purge
+        write !,"Rebuilding cache for repository: ",repoName
+        // $ztimestamp rather than $zhorolog: the latter resets at midnight, making the delta negative.
+        set start = $ztimestamp
+        set sc = serverDef.BuildCache(1, 1)  // purge=1, verbose=1
+        $$$ThrowOnError(sc)
+
+        set end = $ztimestamp
+        set elapsed = (($piece(end,",",1) - $piece(start,",",1)) * 86400) + ($piece(end,",",2) - $piece(start,",",2))
+        write !,"Cache rebuilt successfully in ", $fnumber(elapsed,"",2), " seconds."
     } else {
         set tName = $$$GetModifier(pCommandInfo,"name")
         set tType = $listget(serverClassList)
@@ -2294,6 +2317,16 @@ ClassMethod ShowModulesForRepository(
     set list("width") = width
     write !
     do ..DisplayModules(.list)
+
+    // pRepoName reaches here straight from -name, so it need not be a repository that exists.
+    if '##class(%IPM.Repo.Definition).ServerDefinitionKeyExists(pRepoName) {
+        quit
+    }
+    set server = ##class(%IPM.Repo.Definition).ServerDefinitionKeyOpen(pRepoName,,.tSC)
+    $$$ThrowOnError(tSC)
+    if server.%IsA("%IPM.Repo.Filesystem.Definition") && (server.CacheLastRebuilt '= "") {
+        write !,"Last cache rebuild for '", pRepoName, "' was on ", server.CacheLastRebuilt, " (UTC). If needed, run 'repo -n "_pRepoName_" -rebuild-cache' to refresh the cache.",!
+    }
 }
 
 Query SourceControlClasses() As %SQLQuery(ROWSPEC = "ID:%String,Name:%String") [ SqlProc ]
@@ -2510,6 +2543,29 @@ ClassMethod CheckModuleNamespace() As %Status
     quit $$$OK
 }
 
+/// Rebuild one filesystem repository's cache ahead of a dependency search.
+/// Best effort: a failure warns rather than failing the install, since the existing cache is still
+/// usable, it just might not list modules added since the last rebuild.
+/// Announcement is written once, only when there is a repository to rebuild.
+ClassMethod RebuildFilesystemRepoCache(
+	repoDef As %IPM.Repo.Filesystem.Definition,
+	ByRef announced As %Boolean = 0) [ Internal, Private ]
+{
+    // A root that is not currently mounted is skipped rather than reported: BuildCache refuses it,
+    // and an install has no business failing or warning over an unrelated repository.
+    if 'repoDef.GetPackageService().IsAvailable() {
+        quit
+    }
+    if 'announced {
+        write !,"Rebuilding cache(s) for filesystem repo(s)..."
+        set announced = 1
+    }
+    set sc = repoDef.BuildCache(0, 0)  // purge=0, verbose=0
+    if $$$ISERR(sc) {
+        do ##class(%IPM.General.LogManager).Warning("Could not rebuild cache for filesystem repository '"_repoDef.Name_"': "_$system.Status.GetOneErrorText(sc),1)
+    }
+}
+
 ClassMethod Install(
 	ByRef pCommandInfo,
 	pLog As %IPM.General.AbstractHistory = "") [ Internal ]
@@ -2543,6 +2599,29 @@ ClassMethod Install(
             $$$ThrowStatus($$$ERROR($$$GeneralError, "No repositories are configured and enabled in this namespace."))
         }
 
+        // Rebuild the filesystem repository caches before searching, so modules added since the last
+        // rebuild are discoverable.
+        set announced = 0
+        if (tRegistry '= "") {
+            set tRepoDef = ##class(%IPM.Repo.Definition).ServerDefinitionKeyOpen(tRegistry,,.tSC)
+            if $$$ISERR(tSC) {
+                $$$ThrowStatus($$$ERROR($$$GeneralError,$$$FormatText("No repository with name '%1' exists in this namespace.",tRegistry)))
+            }
+            if tRepoDef.%IsA("%IPM.Repo.Filesystem.Definition") {
+                do ..RebuildFilesystemRepoCache(tRepoDef,.announced)
+            }
+        } else {
+            set tFsRepoResult = ##class(%SQL.Statement).%ExecDirect(,"SELECT ID FROM %IPM_Repo_Filesystem.Definition WHERE Enabled = 1")
+            $$$ThrowSQLIfError(tFsRepoResult.%SQLCODE,tFsRepoResult.%Message)
+            while tFsRepoResult.%Next(.tFetchSC) {
+                $$$ThrowOnError(tFetchSC)
+                set tFsRepo = ##class(%IPM.Repo.Filesystem.Definition).%OpenId(tFsRepoResult.%Get("ID"),,.tOpenSC)
+                $$$ThrowOnError(tOpenSC)
+                do ..RebuildFilesystemRepoCache(tFsRepo,.announced)
+            }
+            $$$ThrowOnError(tFetchSC)
+        }
+
         set tVersion = $get(pCommandInfo("parameters","version"))
         set tKeywords = $$$GetModifier(pCommandInfo,"keywords")
         set tForce = $$$HasModifier(pCommandInfo,"force")

src/cls/IPM/Repo/Filesystem/Cache.cls

@@ -5,16 +5,33 @@ Class %IPM.Repo.Filesystem.Cache Extends (%Persistent, %IPM.General.ModuleInfo)
 
 Parameter DEFAULTGLOBAL = "^IPM.Repo.Filesystem.Cache";
 
-Index CacheItemIndex On (Root, SubDirectory) [ Data = LastModified, Unique ];
+/// Uniqueness constraint only: one cache entry per directory under a given root.
+Index CacheItemIndex On (Root, SubDirectoryHash) [ Unique ];
 
 Property Root As %String(MAXLEN = 260) [ Required ];
 
 ForeignKey RootFK(Root) References %IPM.Repo.Filesystem.Definition(RootIndex) [ OnDelete = cascade ];
 
-Property SubDirectory As %String(MAXLEN = 260);
+Property SubDirectory As %String(MAXLEN = "");
+
+/// SHA-1 of SubDirectory as uppercase hex, giving the unique index a fixed 40-character subscript.
+/// SubDirectory itself cannot be one: a path can exceed the combined subscript budget, and %String's
+/// SQLUPPER collation reports "mods/Foo" and "mods/foo" as the same key when they are two directories.
+Property SubDirectoryHash As %String(MAXLEN = 40) [ Required, SqlComputeCode = {set {*} = ##class(%IPM.Repo.Filesystem.Cache).PathHash({SubDirectory})}, SqlComputed, SqlComputeOnChange = (%%INSERT, %%UPDATE, SubDirectory) ];
 
 Property LastModified As %TimeStamp [ Required ];
 
+/// SHA-1 content hash (lowercase hex) of the module.xml file this entry was built from.
+/// Content, not modification time: mtime is unreliable across container bind mounts, where it can
+/// be coarse, stale, or never propagate at all.
+Property ContentHash As %String(MAXLEN = 40);
+
+/// Indicates if full manifest has been populated (deferred loading optimization).
+/// When 0: Only metadata (Name, Version) is cached; Manifest stream is empty.
+/// When 1: Full <Module> XML has been loaded into Manifest property.
+/// Use LoadManifestForCacheEntry() to populate on-demand when needed.
+Property ManifestLoaded As %Boolean [ InitialExpression = 0 ];
+
 /// Full module manifest
 Property Manifest As %Stream.GlobalCharacter;
 
@@ -25,8 +42,22 @@ Property SemVer As %String(MAXLEN = 512) [ SqlComputeCode = {set {*} = ##class(%
 
 Index RootNameVersion On (Root, Name, VersionString) [ Unique ];
 
+ClassMethod PathHash(subDirectory As %String) As %String [ CodeMode = expression ]
+{
+##class(%xsd.hexBinary).LogicalToXSD($system.Encryption.SHA1Hash(subDirectory))
+}
+
 ClassMethod %OnBeforeBuildIndices(ByRef indexlist As %String(MAXLEN="") = "") As %Status [ Private, ServerOnly = 1 ]
 {
+    if (indexlist [ "CacheItemIndex") || (indexlist = "") {
+        // CacheItemIndex is unique on SubDirectoryHash, so rows carried over from a release that
+        // predates that property would all index under the same empty value. A cache row is derived
+        // data. Drop them and let the next build re-parse the manifests.
+        set tRes = ##class(%SQL.Statement).%ExecDirect(,"delete from %IPM_Repo_Filesystem.Cache where SubDirectoryHash is null or SubDirectoryHash = ''")
+        if tRes.%SQLCODE < 0 {
+            quit $$$ERROR($$$SQLCode,tRes.%SQLCODE,tRes.%Message)
+        }
+    }
     if (indexlist [ "SemVer") || (indexlist = "") {
         // Force recomputation of SemVer property if index needs to be rebuilt (i.e., because structure has changed)
         set tRes = ##class(%SQL.Statement).%ExecDirect(,"update %IPM_Repo_Filesystem.Cache set SemVer = ''")
@@ -113,6 +144,68 @@ Method HandleSaveError(pSC As %Status) As %Status
     quit tSC
 }
 
+/// Open a cache entry and validate it against the filesystem
+/// If the cached entry is stale (ContentHash differs from the hash of the module.xml on disk),
+/// re-parse the module.xml and update the cache entry
+/// Returns the cache object (possibly refreshed) or throws error if not found
+ClassMethod RootNameVersionOpenValidated(
+	root As %String,
+	name As %String,
+	versionString As %String,
+	concurrency As %Integer = -1,
+	Output status As %Status) As %IPM.Repo.Filesystem.Cache
+{
+    set status = $$$OK
+    set cacheObj = ""
+
+    try {
+        // Open the cache entry
+        set cacheObj = ..RootNameVersionOpen(root, name, versionString, concurrency, .status)
+        if $$$ISERR(status) {
+            quit
+        }
+
+        set dirPath = ##class(%File).NormalizeFilename(cacheObj.SubDirectory, cacheObj.Root)
+        set filePath = ##class(%File).NormalizeFilename("module.xml", dirPath)
+        if '##class(%File).Exists(filePath) {
+            // Report it but leave the row alone. The absence may be transient, such as a source
+            // control sync in flight or an unmounted share. Pruning belongs to CleanupStaleEntries,
+            // which only runs after a full walk has established the file is really gone.
+            set cacheObj = ""
+            set status = $$$ERROR($$$GeneralError, "module.xml not found in " _ dirPath)
+            quit
+        }
+
+        set currentHash = $$$lcase(##class(%File).SHA1Hash(filePath, 1))
+        if (cacheObj.ContentHash '= currentHash) {
+            // An unsaved row still holds the old Name and Version, so the index re-check below would
+            // pass on stale data.
+            if '##class(%IPM.Repo.Filesystem.Definition).RefreshCacheEntry(cacheObj, filePath, currentHash) {
+                set message = filePath_" now describes a module already cached under "_root
+                set cacheObj = ""
+                set status = $$$ERROR($$$GeneralError, message)
+                quit
+            }
+
+            // The re-parse can change Name or Version, so the entry may no longer be the module that
+            // was asked for. Returning it would install something other than what the caller resolved.
+            // Re-check through the index rather than comparing properties, so the same key semantics
+            // apply on both sides of the comparison as on the original lookup.
+            if '..RootNameVersionExists(root, name, versionString, .stillId) || (stillId '= cacheObj.%Id()) {
+                set message = filePath_" now describes "_cacheObj.Name_" "_cacheObj.VersionString_", not "_name_" "_versionString
+                set cacheObj = ""
+                set status = $$$ERROR($$$GeneralError, message)
+                quit
+            }
+        }
+    } catch ex {
+        set status = ex.AsStatus()
+        set cacheObj = ""
+    }
+
+    quit cacheObj
+}
+
 Storage Default
 {
 <Data name="CacheDefaultData">
@@ -164,6 +257,15 @@ Storage Default
 <Value name="16">
 <Value>DisplayName</Value>
 </Value>
+<Value name="17">
+<Value>ContentHash</Value>
+</Value>
+<Value name="18">
+<Value>ManifestLoaded</Value>
+</Value>
+<Value name="19">
+<Value>SubDirectoryHash</Value>
+</Value>
 </Data>
 <DataLocation>^IPM.Repo.Filesystem.CacheD</DataLocation>
 <DefaultData>CacheDefaultData</DefaultData>