Merge pull request #35504 from overleaf/bg-png2pdf-on-slow-images-only

run png2pdf on slow images only

GitOrigin-RevId: 2d0ecd35bfcfc308773502c28eeb5a722055a5a9
This commit is contained in:
Brian Gough
2026-09-17 11:29:12 +00:00
committed by Copybot
parent aad50452af
commit 9ff7e66b99
7 changed files with 540 additions and 23 deletions
+18
View File
@@ -23,6 +23,7 @@ import SafeReader from './SafeReader.js'
import LatexMetrics from './LatexMetrics.js'
import { callbackifyMultiResult } from '@overleaf/promise-utils'
import * as HistoryResourceWriter from './HistoryResourceWriter.js'
import Png2Pdf from './Png2Pdf.js'
const { downloadLatestCompileCache, downloadOutputDotSynctexFromCompileCache } =
CLSICacheHandler
@@ -280,6 +281,23 @@ async function doCompile(request, stats, timings) {
timings.compile = Date.now() - compileStart
// Record the PNGs this compile flagged as "slow" so the next sync can convert
// them to PDFs (see HistoryResourceWriter).
if (request.isCompileFromHistory && request.png2pdf && Png2Pdf.isEnabled()) {
try {
const slowPngs = stats.latexmk?.['latexmk-png-slow'] || []
await HistoryResourceWriter.saveSlowPngList(
Path.basename(compileDir),
slowPngs
)
} catch (err) {
logger.warn(
{ err, projectId, userId },
'failed to save png2pdf slow-png list'
)
}
}
logger.debug(
{
projectId: request.project_id,
+113 -20
View File
@@ -71,14 +71,56 @@ export async function clearCache(projectId, userId, cacheKey) {
/**
* @param {string} cacheKey
* @return {{ dir: string, path: string, resyncPath: string }}
* @return {{ dir: string, path: string, resyncPath: string, slowPngPath: string }}
*/
function snapshotPath(cacheKey) {
const dir = Path.join(Settings.path.clsiCacheDir, cacheKey)
const path = Path.join(dir, 'history.json.gz')
const resyncPath = Path.join(dir, 'history-resync.json.gz')
return { dir, path, resyncPath }
// The set of PNGs the last compile flagged as slow (needing png2pdf). Written
// after the compile (the info is only available then), read on the next sync.
const slowPngPath = Path.join(dir, 'png2pdf-slow.json')
return { dir, path, resyncPath, slowPngPath }
}
/**
* Persist the list of "slow" PNG paths (those that could not be fast-copied and
* would benefit from png2pdf conversion) learned from the compile that just ran,
* so the next sync can convert only those files. Best-effort: a failure here just
* means the next sync falls back to the previous list (or converts nothing new).
*
* @param {string} cacheKey
* @param {string[]} slowPngs
* @return {Promise<void>}
*/
export async function saveSlowPngList(cacheKey, slowPngs) {
const { dir, slowPngPath } = snapshotPath(cacheKey)
const tmp = slowPngPath + '~'
await fs.promises.mkdir(dir, { recursive: true })
await fs.promises.writeFile(tmp, JSON.stringify(slowPngs))
await fs.promises.rename(tmp, slowPngPath)
}
/**
* @param {string} cacheKey
* @return {Promise<string[]>}
*/
async function loadSlowPngList(cacheKey) {
const { slowPngPath } = snapshotPath(cacheKey)
try {
const blob = await fs.promises.readFile(slowPngPath, 'utf-8')
const list = JSON.parse(blob)
return Array.isArray(list) ? list : []
} catch (err) {
if (!isENOENT(err)) {
logger.warn(
{ err, cacheKey },
'compile from cache: cannot read slow-png list'
)
}
return []
}
}
/**
@@ -477,6 +519,55 @@ export async function syncResourcesToDisk(
const pngModeChanged = lastPng2pdf !== request.png2pdf
const blobStore = new BlobStore(
request.historyId,
request.filestoreBlobPrefix,
request.clsiPerfVariant,
globalBlobs
)
// Decide which PNGs to convert. A PNG is converted when a previous compile
// flagged it as "slow" and it is large enough to be worth converting; keeping
// that decision here means the sync loop below just checks membership.
const png2pdfActive = request.png2pdf && Png2Pdf.isEnabled()
const slowPngs = new Set(png2pdfActive ? await loadSlowPngList(cacheKey) : [])
const shouldConvert = new Set()
for (const path of snapshot.getFilePathnames()) {
if (!isPng(path) || !slowPngs.has(path)) continue
// Avoid doing unnecessary work converting small PNGs.
const fileSize = snapshot.getFile(path)?.getByteLength() || 0
if (pngBelowSizeThreshold(fileSize)) {
Metrics.inc('png2pdf-skipped-small')
continue
}
shouldConvert.add(path)
}
// On a png2pdf mode switch, also re-serve PNGs that a previous compile already
// optimised. Once converted, a PNG is no longer flagged slow (it is included
// as a PDF), so it drops off the slow-list; without this it would revert to
// the original when toggling png2pdf off and back on. The optimised variant is
// served from the <cachePath>.opt cache, so this is a cheap local cache stat.
if (png2pdfActive && pngModeChanged) {
const candidates = snapshot
.getFilePathnames()
.filter(path => isPng(path) && !shouldConvert.has(path))
await promiseMapSettledWithLimit(
Settings.parallelFileDownloads,
candidates,
async path => {
const hash = snapshot.getFile(path)?.getHash()
if (!hash) return
const url = blobStore.getBlobURL(hash).href
const cached = await UrlCache.promises.isConversionCached(
projectId,
url,
new Date(0)
)
if (cached) shouldConvert.add(path)
}
)
}
const changedPaths = []
if (fullSync) {
changedPaths.push(...snapshot.getFilePathnames())
@@ -511,6 +602,23 @@ export async function syncResourcesToDisk(
for (const path of snapshot.getFilePathnames()) {
if (!entriesDepthFirst.has(path)) dedupe.add(path)
}
// Include PNGs known to be slow for png2pdf conversion. The presence of
// an optimised (.opt) cache entry means the conversion was already
// attempted (success or failure), so we skip those and never retry.
// The .opt cache is keyed by content hash, so a new PNG at the same path
// has no entry and is attempted.
for (const path of shouldConvert) {
if (dedupe.has(path)) continue
const hash = snapshot.getFile(path)?.getHash()
if (!hash) continue
const url = blobStore.getBlobURL(hash).href
const attempted = await UrlCache.promises.isConversionCached(
projectId,
url,
new Date(0)
)
if (!attempted) dedupe.add(path)
}
changedPaths.push(...dedupe)
logger.debug(
{ projectId, userId, cacheKey, changedPaths },
@@ -518,12 +626,6 @@ export async function syncResourcesToDisk(
)
}
const blobStore = new BlobStore(
request.historyId,
request.filestoreBlobPrefix,
request.clsiPerfVariant,
globalBlobs
)
const loadEagerStart = performance.now()
await snapshot.loadFiles('eager', blobStore)
timings.snapshotLoadEager = Math.ceil(performance.now() - loadEagerStart)
@@ -571,7 +673,6 @@ export async function syncResourcesToDisk(
'utf-8'
)
} else {
const fileSize = file.getByteLength() || 0
const hash = file.getHash()
if (!hash) {
throw new OError('unexpected file without content and hash', { path })
@@ -585,17 +686,9 @@ export async function syncResourcesToDisk(
try {
const fallbackURL = null // no fallback
const lastModified = new Date(0) // content is static
const isConvertiblePng =
request.png2pdf && Png2Pdf.isEnabled() && isPng(path)
// Avoid doing unnecessary work converting small PNGs
const skipConversion =
isConvertiblePng && pngBelowSizeThreshold(fileSize)
if (skipConversion) {
Metrics.inc('png2pdf-skipped-small')
}
// Take the conversion path for PNGs to be optimised
if (isConvertiblePng && !skipConversion) {
// PNG files go through a batch conversion process first.
// PNGs selected for conversion go through a batch conversion process
// first (see shouldConvert above).
if (shouldConvert.has(path)) {
const toConvert = await UrlCache.promises.downloadUrlToFile(
projectId,
url,
+34 -3
View File
@@ -102,7 +102,40 @@ const LATEX_MK_METRICS_STDOUT = [
],
]
/**
* Normalise a filename from latexmk stderr to a project-relative path so it can
* be matched against history snapshot pathnames. Strips a leading `/compile/`
* (see parseFdbContent) and a leading `./`.
*
* @param {string} filename
* @returns {string}
*/
function normalizeImageFilename(filename) {
return filename.replace(/^\/compile\//, '').replace(/^\.\//, '')
}
// A PNG that could not be fast-copied, and the reason it was included the slow
// way. Group 1 is the category, group 2 is the filename.
const PNG_COPY_SKIPPED_RE =
/^PNG copy skipped \((alpha|gamma|palette|interlaced|other)\): (.*)$/gm
const LATEX_MK_METRICS_STDERR = [
// The project-relative paths of PNGs that could not be fast-copied and were
// therefore included the slow way (transparency/alpha/gamma/palette/interlaced).
// These are exactly the PNGs that benefit from png2pdf conversion.
[
'latexmk-png-slow',
s => {
// Only the filename (group 2) matters here; the category is ignored.
const normalizedFiles = Array.from(
s.matchAll(PNG_COPY_SKIPPED_RE),
match => normalizeImageFilename(match[2])
)
// deduplicate
const files = [...new Set(normalizedFiles)]
return files.length > 0 ? files : null
},
],
[
'latexmk-img-times',
s => {
@@ -113,9 +146,7 @@ const LATEX_MK_METRICS_STDERR = [
pngCategoriesByFile.set(filename, 'fast-copy')
}
const pngCopySkipMatches = s.matchAll(
/^PNG copy skipped \((alpha|gamma|palette|interlaced)\): (.*)$/gm
)
const pngCopySkipMatches = s.matchAll(PNG_COPY_SKIPPED_RE)
for (const match of pngCopySkipMatches) {
const category = match[1]
const filename = match[2]
+28
View File
@@ -13,6 +13,7 @@
import UrlFetcher from './UrlFetcher.js'
import Settings from '@overleaf/settings'
import logger from '@overleaf/logger'
import fs from 'node:fs'
import Path from 'node:path'
import { callbackify } from 'node:util'
@@ -185,6 +186,32 @@ async function commitConversion(conversionPath, cachePath, destPath) {
timer.done()
}
/**
* Whether an optimised (png2pdf-converted) variant of a url is already cached.
* The `.opt` entry is written by commitConversion even when the conversion
* failed (holding the original bytes), so its presence means a conversion has
* already been attempted for this content and should not be attempted again.
*
* @param {string} projectId
* @param {string} url
* @param {Date} lastModified
* @return {Promise<boolean>}
*/
async function isConversionCached(projectId, url, lastModified) {
const cachePath = getCachePath(projectId, url, lastModified) + '.opt'
try {
await fs.promises.access(cachePath)
return true
} catch (err) {
if (err.code === 'ENOENT') return false
logger.warn(
{ err, projectId, url, cachePath },
'failure checking cache for converted file'
)
return false
}
}
export default {
clearProject: callbackify(clearProject),
createProjectDir: callbackify(createProjectDir),
@@ -195,5 +222,6 @@ export default {
createProjectDir,
downloadUrlToFile,
commitConversion,
isConversionCached,
},
}
@@ -150,6 +150,18 @@ describe('CompileManager', () => {
downloadOutputDotSynctexFromCompileCache: sinon.stub().resolves(),
}
ctx.HistoryResourceWriter = {
syncResourcesToDisk: sinon.stub().resolves({
resourceList: ctx.resources,
baseHistoryVersion: undefined,
}),
saveSlowPngList: sinon.stub().resolves(),
}
ctx.Png2Pdf = {
isEnabled: sinon.stub().returns(true),
}
ctx.LatexMetrics = { enableLatexMkMetrics: sinon.stub() }
ctx.StatsManager = { sampleRequest: sinon.stub().returns(false) }
@@ -215,6 +227,15 @@ describe('CompileManager', () => {
default: ctx.CLSICacheHandler,
}))
vi.doMock(
'../../../app/js/HistoryResourceWriter',
() => ctx.HistoryResourceWriter
)
vi.doMock('../../../app/js/Png2Pdf', () => ({
default: ctx.Png2Pdf,
}))
vi.doMock('../../../app/js/LatexMetrics', () => ({
default: ctx.LatexMetrics,
}))
@@ -472,6 +493,48 @@ describe('CompileManager', () => {
expect(ctx.fsPromises.rmdir).to.have.been.calledWith(ctx.compileDir)
})
})
describe('with slow pngs from latexmk metrics', () => {
beforeEach(ctx => {
ctx.request.isCompileFromHistory = true
ctx.request.png2pdf = true
})
it('should save the slow png list for the next compile', async ctx => {
const stats = {
latexmk: {
'latexmk-png-slow': ['images/alpha.png', 'images/palette.png'],
},
}
await ctx.CompileManager.promises.doCompileWithLock(
ctx.request,
stats,
{}
)
expect(
ctx.HistoryResourceWriter.saveSlowPngList
).to.have.been.calledWith(`${ctx.projectId}-${ctx.userId}`, [
'images/alpha.png',
'images/palette.png',
])
})
it('should not fail compile when saving the slow png list fails', async ctx => {
ctx.HistoryResourceWriter.saveSlowPngList.rejects(
new Error('save failed')
)
await expect(
ctx.CompileManager.promises.doCompileWithLock(
ctx.request,
{ latexmk: { 'latexmk-png-slow': ['images/alpha.png'] } },
{}
)
).to.be.fulfilled
})
})
})
describe('clearProject', () => {
@@ -0,0 +1,244 @@
import Path from 'node:path'
import os from 'node:os'
import fs from 'node:fs'
import sinon from 'sinon'
import { vi, describe, beforeEach, afterEach, it, expect } from 'vitest'
const MODULE_PATH = Path.join(
import.meta.dirname,
'../../../app/js/HistoryResourceWriter'
)
describe('HistoryResourceWriter', function () {
beforeEach(async function (ctx) {
ctx.tmp = fs.mkdtempSync(Path.join(os.tmpdir(), 'clsi-hrw-'))
ctx.projectId = 'p1'
ctx.userId = 'u1'
ctx.cacheKey = `${ctx.projectId}-${ctx.userId}`
ctx.compileDir = Path.join(ctx.tmp, 'compiles', ctx.cacheKey)
fs.mkdirSync(ctx.compileDir, { recursive: true })
ctx.Settings = {
path: { clsiCacheDir: Path.join(ctx.tmp, 'cache') },
parallelFileDownloads: 1,
png2pdfMinFileSizeBytes: 1024 * 1024,
apis: {
filestore: { url: 'http://filestore.test' },
clsiPerf: { host: 'perf.test' },
},
}
// Models the on-disk cache: `optCache` holds the urls that have an optimised
// (.opt) entry. A conversion download misses the first time (returns a
// handle and records the .opt entry) and hits thereafter (returns
// undefined, i.e. no reconversion). Every download writes the destination
// file so the compile dir reflects what a real download would leave behind.
ctx.optCache = new Set()
ctx.downloadUrlToFile = sinon
.stub()
.callsFake(
async (projectId, url, fallback, destPath, lastModified, suffix) => {
await fs.promises.writeFile(destPath, 'png-bytes')
if (suffix) {
if (ctx.optCache.has(url)) return undefined
ctx.optCache.add(url)
return {
conversionPath: destPath + '.conv',
cachePath: destPath + '.opt',
destPath,
}
}
return undefined
}
)
ctx.commitConversion = sinon.stub().resolves()
ctx.isConversionCached = sinon
.stub()
.callsFake(async (projectId, url) => ctx.optCache.has(url))
ctx.UrlCache = {
promises: {
createProjectDir: sinon.stub().resolves(),
downloadUrlToFile: ctx.downloadUrlToFile,
commitConversion: ctx.commitConversion,
isConversionCached: ctx.isConversionCached,
},
getProjectCacheDir: sinon
.stub()
.returns(Path.join(ctx.tmp, 'cache', ctx.projectId)),
}
ctx.convertPng = sinon.stub().resolves()
ctx.Png2Pdf = {
isEnabled: () => true,
convertPngFilesInCacheDir: ctx.convertPng,
}
vi.doMock('@overleaf/settings', () => ({ default: ctx.Settings }))
vi.doMock('@overleaf/logger', () => ({
default: { debug: sinon.stub(), warn: sinon.stub(), err: sinon.stub() },
}))
vi.doMock('@overleaf/metrics', () => ({ default: { inc: sinon.stub() } }))
vi.doMock('@overleaf/fetch-utils', () => ({
fetchString: sinon.stub(),
RequestFailedError: class extends Error {},
}))
vi.doMock('../../../app/js/UrlCache', () => ({ default: ctx.UrlCache }))
vi.doMock('../../../app/js/Png2Pdf', () => ({ default: ctx.Png2Pdf }))
vi.doMock('../../../app/js/TikzManager', () => ({
default: { writeOutputFileIfNeeded: sinon.stub().resolves() },
}))
vi.doMock('../../../app/js/DraftModeManager', () => ({
default: { PREFIX: '' },
}))
vi.doMock('../../../app/js/CLSICacheHandler', () => ({
default: { downloadHistorySnapshot: sinon.stub().resolves(false) },
}))
vi.doMock('../../../app/js/ResourceWriter', () => ({
default: { isExtraneousFile: () => false },
}))
vi.doMock('../../../app/js/Metrics', () => ({
default: {
shouldSkipMetrics: () => true,
snapshotApplyAllDurationSeconds: { observe: sinon.stub() },
snapshotLoadEagerDurationSeconds: { observe: sinon.stub() },
},
}))
// Use the real overleaf-editor-core Snapshot/File; the fixtures below are
// representable without any blob loading (an inline string file and a
// hash+byteLength binary file), so no blobStore access is needed.
ctx.HistoryResourceWriter = await import(MODULE_PATH)
// Real overleaf-editor-core raw FileMap: an object keyed by pathname.
// { hash, byteLength } -> BinaryFileData (the PNG); { content } ->
// StringFileData (the root doc). The hash must be a 40-char hex string.
ctx.pngHash = '0123456789012345678901234567890123456789'
ctx.rawSnapshot = {
files: {
'fig.png': { hash: ctx.pngHash, byteLength: 2 * 1024 * 1024 },
'main.tex': { content: 'hello' },
},
}
ctx.makeRequest = (overrides = {}) => ({
project_id: ctx.projectId,
user_id: ctx.userId,
baseHistoryVersion: 0,
rawSnapshot: ctx.rawSnapshot,
globalBlobs: [],
rawChangeOperations: [],
populateClsiCache: false,
png2pdf: true,
historyId: 'hist1',
filestoreBlobPrefix: '',
clsiPerfVariant: '',
draft: false,
rootResourcePath: 'main.tex',
compileGroup: 'standard',
...overrides,
})
ctx.sync = (overrides = {}) =>
ctx.HistoryResourceWriter.syncResourcesToDisk(
ctx.projectId,
ctx.userId,
ctx.makeRequest(overrides),
ctx.compileDir,
{},
{}
)
})
afterEach(function (ctx) {
fs.rmSync(ctx.tmp, { recursive: true, force: true })
vi.resetModules()
vi.clearAllMocks()
})
describe('saveSlowPngList', function () {
it('writes the list to png2pdf-slow.json in the cache dir', async function (ctx) {
await ctx.HistoryResourceWriter.saveSlowPngList(ctx.cacheKey, [
'a.png',
'b.png',
])
const file = Path.join(
ctx.Settings.path.clsiCacheDir,
ctx.cacheKey,
'png2pdf-slow.json'
)
expect(JSON.parse(fs.readFileSync(file, 'utf-8'))).to.deep.equal([
'a.png',
'b.png',
])
})
})
describe('slow-list-gated conversion', function () {
it('does not convert PNGs until they are known to be slow, then converts once', async function (ctx) {
// Sync 1: no slow-list yet -> the PNG is downloaded normally, not converted.
await ctx.sync()
expect(ctx.downloadUrlToFile.callCount).to.equal(1)
expect(ctx.downloadUrlToFile.getCall(0).args[3]).to.equal(
Path.join(ctx.compileDir, 'fig.png')
)
expect(ctx.downloadUrlToFile).to.have.been.calledOnce
expect(ctx.downloadUrlToFile).to.have.been.calledWithMatch(
ctx.projectId,
sinon.match.string,
null,
Path.join(ctx.compileDir, 'fig.png'),
sinon.match.date
)
expect(ctx.convertPng.called).to.equal(false)
// A compile then flags fig.png as slow.
await ctx.HistoryResourceWriter.saveSlowPngList(ctx.cacheKey, ['fig.png'])
// Sync 2: the newly-slow PNG is pulled in and routed through conversion.
ctx.downloadUrlToFile.resetHistory()
ctx.convertPng.resetHistory()
ctx.commitConversion.resetHistory()
await ctx.sync()
expect(ctx.downloadUrlToFile.callCount).to.equal(1)
expect(ctx.downloadUrlToFile.getCall(0).args[5]).to.equal(ctx.cacheKey)
expect(ctx.convertPng.calledOnce).to.equal(true)
expect(ctx.commitConversion.calledOnce).to.equal(true)
// Sync 3: already attempted -> not converted again (attempt-once).
ctx.downloadUrlToFile.resetHistory()
ctx.convertPng.resetHistory()
await ctx.sync()
expect(ctx.convertPng.called).to.equal(false)
expect(ctx.downloadUrlToFile.called).to.equal(false)
})
it('re-serves an already-optimised PNG after a png2pdf mode switch', async function (ctx) {
// Get fig.png converted so it has an .opt cache entry.
await ctx.HistoryResourceWriter.saveSlowPngList(ctx.cacheKey, ['fig.png'])
await ctx.sync()
expect(ctx.convertPng.calledOnce).to.equal(true)
// The next compile no longer flags it slow (it is now included as a PDF),
// so it drops off the slow-list.
await ctx.HistoryResourceWriter.saveSlowPngList(ctx.cacheKey, [])
// Toggle png2pdf off: the PNG reverts to the original on disk (downloaded
// without the conversion cacheKey suffix).
ctx.downloadUrlToFile.resetHistory()
ctx.convertPng.resetHistory()
await ctx.sync({ png2pdf: false })
expect(ctx.downloadUrlToFile.calledOnce).to.equal(true)
expect(ctx.downloadUrlToFile.getCall(0).args[5]).to.equal(undefined)
// Toggle png2pdf back on: even though fig.png is no longer on the
// slow-list, its cached .opt entry is re-served (routed through the
// conversion path with the cacheKey), so it is restored to the optimised
// variant rather than reverting to the original - and without a new
// conversion.
ctx.downloadUrlToFile.resetHistory()
ctx.convertPng.resetHistory()
await ctx.sync({ png2pdf: true })
expect(ctx.downloadUrlToFile.calledOnce).to.equal(true)
expect(ctx.downloadUrlToFile.getCall(0).args[5]).to.equal(ctx.cacheKey)
expect(ctx.convertPng.called).to.equal(false)
})
})
})
@@ -221,5 +221,45 @@ describe('LatexMetrics', function () {
])
})
})
describe('latexmk-png-slow', function () {
it('should list files that were PNG copy skipped', function (ctx) {
const output = {
stderr:
'PNG copy: fast.png\n' +
'PNG copy skipped (alpha): images/alpha.png\n' +
'PNG copy skipped (palette): palette.png\n' +
'PNG copy skipped (other): other.png\n',
}
addLatexMkMetrics(output, ctx.stats)
expect(ctx.stats.latexmk['latexmk-png-slow']).to.deep.equal([
'images/alpha.png',
'palette.png',
'other.png',
])
})
it('should not include fast PNG copy lines', function (ctx) {
const output = { stderr: 'PNG copy: fast.png\n' }
addLatexMkMetrics(output, ctx.stats)
expect(ctx.stats.latexmk).to.not.have.property('latexmk-png-slow')
})
it('should normalize a leading /compile/ and ./', function (ctx) {
const output = {
stderr:
'PNG copy skipped (gamma): /compile/deep/img.png\n' +
'PNG copy skipped (interlaced): ./local.png\n',
}
addLatexMkMetrics(output, ctx.stats)
expect(ctx.stats.latexmk['latexmk-png-slow']).to.deep.equal([
'deep/img.png',
'local.png',
])
})
})
})
})