Fix perf regression when checking for changed content

src/lib/content.js

@@ -164,50 +164,46 @@ function resolvePathSymlinks(contentPath) {
  * @param {any} context
  * @param {ContentPath[]} candidateFiles
  * @param {Map<string, number>} fileModifiedMap
- * @returns {{ content: string, extension: string }[]}
+ * @returns {[{ content: string, extension: string }[], Map<string, number>]}
  */
 export function resolvedChangedContent(context, candidateFiles, fileModifiedMap) {
   let changedContent = context.tailwindConfig.content.files
     .filter((item) => typeof item.raw === 'string')
     .map(({ raw, extension = 'html' }) => ({ content: raw, extension }))
 
-  for (let changedFile of resolveChangedFiles(candidateFiles, fileModifiedMap)) {
+  let [changedFiles, mTimesToCommit] = resolveChangedFiles(candidateFiles, fileModifiedMap)
+
+  for (let changedFile of changedFiles) {
     let content = fs.readFileSync(changedFile, 'utf8')
     let extension = path.extname(changedFile).slice(1)
     changedContent.push({ content, extension })
   }
 
-  return changedContent
+  return [changedContent, mTimesToCommit]
 }
 
 /**
  *
  * @param {ContentPath[]} candidateFiles
  * @param {Map<string, number>} fileModifiedMap
- * @returns {Set<string>}
+ * @returns {[Set<string>, Map<string, number>]}
  */
 function resolveChangedFiles(candidateFiles, fileModifiedMap) {
   let paths = candidateFiles.map((contentPath) => contentPath.pattern)
+  let mTimesToCommit = new Map()
 
   let changedFiles = new Set()
   env.DEBUG && console.time('Finding changed files')
   let files = fastGlob.sync(paths, { absolute: true })
   for (let file of files) {
-    let prevModified = fileModifiedMap.has(file) ? fileModifiedMap.get(file) : -Infinity
+    let prevModified = fileModifiedMap.get(file) || -Infinity
     let modified = fs.statSync(file).mtimeMs
 
-    // This check is intentionally >= because we track the last modified time of context dependencies
-    // earier in the process and we want to make sure we don't miss any changes that happen
-    // when a context dependency is also a content dependency
-    // Ideally, we'd do all this tracking at one time but that is a larger refactor
-    // than we want to commit to right now, so this is a decent compromise.
-    // This should be sufficient because file modification times will be off by at least
-    // 1ms (the precision of fstat in Node) in most cases if they exist and were changed.
-    if (modified >= prevModified) {
+    if (modified > prevModified) {
       changedFiles.add(file)
-      fileModifiedMap.set(file, modified)
+      mTimesToCommit.set(file, modified)
     }
   }
   env.DEBUG && console.timeEnd('Finding changed files')
-  return changedFiles
+  return [changedFiles, mTimesToCommit]
 }

src/lib/setupContextUtils.js

@@ -632,6 +632,7 @@ export function getFileModifiedMap(context) {
 
 function trackModified(files, fileModifiedMap) {
   let changed = false
+  let mtimesToCommit = new Map()
 
   for (let file of files) {
     if (!file) continue
@@ -652,10 +653,10 @@ function trackModified(files, fileModifiedMap) {
       changed = true
     }
 
-    fileModifiedMap.set(file, newModified)
+    mtimesToCommit.set(file, newModified)
   }
 
-  return changed
+  return [changed, mtimesToCommit]
 }
 
 function extractVariantAtRules(node) {
@@ -1230,12 +1231,12 @@ export function getContext(
   // If there's already a context in the cache and we don't need to
   // reset the context, return the cached context.
   if (existingContext) {
-    let contextDependenciesChanged = trackModified(
+    let [contextDependenciesChanged, mtimesToCommit] = trackModified(
       [...contextDependencies],
       getFileModifiedMap(existingContext)
     )
     if (!contextDependenciesChanged && !cssDidChange) {
-      return [existingContext, false]
+      return [existingContext, false, mtimesToCommit]
     }
   }
 
@@ -1270,7 +1271,7 @@ export function getContext(
     userConfigPath,
   })
 
-  trackModified([...contextDependencies], getFileModifiedMap(context))
+  let [, mtimesToCommit] = trackModified([...contextDependencies], getFileModifiedMap(context))
 
   // ---
 
@@ -1285,5 +1286,5 @@ export function getContext(
 
   contextSourcesMap.get(context).add(sourcePath)
 
-  return [context, true]
+  return [context, true, mtimesToCommit]
 }

src/lib/setupTrackingContext.js

@@ -1,3 +1,5 @@
+// @ts-check
+
 import fs from 'fs'
 import LRU from 'quick-lru'
 
@@ -101,7 +103,7 @@ export default function setupTrackingContext(configOrPath) {
         }
       }
 
-      let [context] = getContext(
+      let [context, , mTimesToCommit] = getContext(
         root,
         result,
         tailwindConfig,
@@ -110,6 +112,8 @@ export default function setupTrackingContext(configOrPath) {
         contextDependencies
       )
 
+      let fileModifiedMap = getFileModifiedMap(context)
+
       let candidateFiles = getCandidateFiles(context, tailwindConfig)
 
       // If there are no @tailwind or @apply rules, we don't consider this CSS file or it's
@@ -118,28 +122,49 @@ export default function setupTrackingContext(configOrPath) {
       // because it's impossible for a layer in one file to end up in the actual @tailwind rule
       // in another file since independent sources are effectively isolated.
       if (tailwindDirectives.size > 0) {
-        let fileModifiedMap = getFileModifiedMap(context)
-
         // Add template paths as postcss dependencies.
         for (let contentPath of candidateFiles) {
           for (let dependency of parseDependency(contentPath)) {
             registerDependency(dependency)
           }
         }
 
-        for (let changedContent of resolvedChangedContent(
+        let [changedContent, contentMTimesToCommit] = resolvedChangedContent(
           context,
           candidateFiles,
           fileModifiedMap
-        )) {
-          context.changedContent.push(changedContent)
+        )
+
+        for (let content of changedContent) {
+          context.changedContent.push(content)
+        }
+
+        // Add the mtimes of the content files to the commit list
+        // We can overwrite the existing values because unconditionally
+        // This is because:
+        // 1. Most of the files here won't be in the map yet
+        // 2. If they are that means it's a context dependency
+        // and we're reading this after the context. This means
+        // that the mtime we just read is strictly >= the context
+        // mtime. Unless the user / os is doing something weird
+        // in which the mtime would be going backwards. If that
+        // happens there's already going to be problems.
+        for (let [path, mtime] of contentMTimesToCommit.entries()) {
+          mTimesToCommit.set(path, mtime)
         }
       }
 
       for (let file of configDependencies) {
         registerDependency({ type: 'dependency', file })
       }
 
+      // "commit" the new modified time for all context deps
+      // We do this here because we want content tracking to
+      // read the "old" mtime even when it's a context dependency.
+      for (let [path, mtime] of mTimesToCommit.entries()) {
+        fileModifiedMap.set(path, mtime)
+      }
+
       return context
     }
   }