Skip to content

Commit

Permalink
Fix perf regression when checking for changed content (#10234)
Browse files Browse the repository at this point in the history
* Commit changes to mod time cache all at once

This allows us to track changes in files that are both a context and content dependency in a way that preserves file mod checking optimizations

* fixup
  • Loading branch information
thecrypticace committed Jan 3, 2023
1 parent 7d8eb21 commit ec5136c
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 27 deletions.
26 changes: 11 additions & 15 deletions src/lib/content.js
Expand Up @@ -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]
}
13 changes: 7 additions & 6 deletions src/lib/setupContextUtils.js
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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]
}
}

Expand Down Expand Up @@ -1270,7 +1271,7 @@ export function getContext(
userConfigPath,
})

trackModified([...contextDependencies], getFileModifiedMap(context))
let [, mtimesToCommit] = trackModified([...contextDependencies], getFileModifiedMap(context))

// ---

Expand All @@ -1285,5 +1286,5 @@ export function getContext(

contextSourcesMap.get(context).add(sourcePath)

return [context, true]
return [context, true, mtimesToCommit]
}
37 changes: 31 additions & 6 deletions src/lib/setupTrackingContext.js
@@ -1,3 +1,5 @@
// @ts-check

import fs from 'fs'
import LRU from 'quick-lru'

Expand Down Expand Up @@ -101,7 +103,7 @@ export default function setupTrackingContext(configOrPath) {
}
}

let [context] = getContext(
let [context, , mTimesToCommit] = getContext(
root,
result,
tailwindConfig,
Expand All @@ -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
Expand All @@ -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
}
}
Expand Down

0 comments on commit ec5136c

Please sign in to comment.