fix: load the tags manifest asynchronously (#64563)

## What?
A small update to FileSystemCache to replace sync calls with async.

## Why?

`loadTagsManifest` may be called multiple times per request. Since
`loadTagsManifest` is synchronous it blocks the main thread whilst
reading from the file system which could impact server performance.
Replacing these sync calls with async has no impact for consumers of the
FileSystemCache.

Co-authored-by: JJ Kasper <jj@jjsweb.site>
This commit is contained in:
Imran Sulemanji 2024-05-13 23:53:19 +01:00 committed by GitHub
parent 7725047c89
commit 4128dd2bc0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -94,13 +94,34 @@ export default class FileSystemCache implements CacheHandler {
'fetch-cache', 'fetch-cache',
'tags-manifest.json' 'tags-manifest.json'
) )
this.loadTagsManifest()
this.loadTagsManifestSync()
} }
} }
public resetRequestCache(): void {} public resetRequestCache(): void {}
private loadTagsManifest() { /**
* Load the tags manifest from the file system
*/
private async loadTagsManifest() {
if (!this.tagsManifestPath || !this.fs || tagsManifest) return
try {
tagsManifest = JSON.parse(
await this.fs.readFile(this.tagsManifestPath, 'utf8')
)
} catch (err: any) {
tagsManifest = { version: 1, items: {} }
}
if (this.debug) console.log('loadTagsManifest', tagsManifest)
}
/**
* As above, but synchronous for use in the constructor. This is to
* preserve the existing behaviour when instantiating the cache handler. Although it's
* not ideal to block the main thread it's only called once during startup.
*/
private loadTagsManifestSync() {
if (!this.tagsManifestPath || !this.fs || tagsManifest) return if (!this.tagsManifestPath || !this.fs || tagsManifest) return
try { try {
tagsManifest = JSON.parse( tagsManifest = JSON.parse(
@ -129,7 +150,7 @@ export default class FileSystemCache implements CacheHandler {
// we need to ensure the tagsManifest is refreshed // we need to ensure the tagsManifest is refreshed
// since separate workers can be updating it at the same // since separate workers can be updating it at the same
// time and we can't flush out of sync data // time and we can't flush out of sync data
this.loadTagsManifest() await this.loadTagsManifest()
if (!tagsManifest || !this.tagsManifestPath) { if (!tagsManifest || !this.tagsManifestPath) {
return return
} }
@ -289,7 +310,7 @@ export default class FileSystemCache implements CacheHandler {
} }
if (cacheTags?.length) { if (cacheTags?.length) {
this.loadTagsManifest() await this.loadTagsManifest()
const isStale = cacheTags.some((tag) => { const isStale = cacheTags.some((tag) => {
return ( return (
@ -309,7 +330,7 @@ export default class FileSystemCache implements CacheHandler {
} }
if (data && data?.value?.kind === 'FETCH') { if (data && data?.value?.kind === 'FETCH') {
this.loadTagsManifest() await this.loadTagsManifest()
const combinedTags = [...(tags || []), ...(softTags || [])] const combinedTags = [...(tags || []), ...(softTags || [])]