From 8ee5acd85db2a632ffe7062fa2a22cab7e3eebe4 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 18 Aug 2021 12:24:51 +0200 Subject: [PATCH 1/2] Tweak handling of the `onlyRefs`-option in the `BaseLocalCache` class --- src/core/image_utils.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/core/image_utils.js b/src/core/image_utils.js index 1cfdbb83f1b40..b1a7e1d4eedde 100644 --- a/src/core/image_utils.js +++ b/src/core/image_utils.js @@ -21,7 +21,9 @@ class BaseLocalCache { if (this.constructor === BaseLocalCache) { unreachable("Cannot initialize BaseLocalCache."); } - if (!options || !options.onlyRefs) { + this._onlyRefs = (options && options.onlyRefs) === true; + + if (!this._onlyRefs) { this._nameRefMap = new Map(); this._imageMap = new Map(); } @@ -29,6 +31,9 @@ class BaseLocalCache { } getByName(name) { + if (this._onlyRefs) { + unreachable("Should not call `getByName` method."); + } const ref = this._nameRefMap.get(name); if (ref) { return this.getByRef(ref); @@ -97,10 +102,6 @@ class LocalFunctionCache extends BaseLocalCache { super({ onlyRefs: true }); } - getByName(name) { - unreachable("Should not call `getByName` method."); - } - set(name = null, ref, data) { if (!ref) { throw new Error('LocalFunctionCache.set - expected "ref" argument.'); From 5f25fea0fe6360913dd17106cbe3830eb032a103 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 18 Aug 2021 12:49:01 +0200 Subject: [PATCH 2/2] Re-factor the `LocalTilingPatternCache` to cache by Ref rather than Name (PR 12458 follow-up, issue 13780) This way there cannot be any *incorrect* cache hits, since Refs are guaranteed to be unique. Please note that the reason for caching by Ref rather than doing something along the lines of the `localShadingPatternCache` (which uses a `Map` directly), is that TilingPatterns are streams and those cannot be cached on the `XRef`-instance (this way we avoid unnecessary parsing). --- src/core/evaluator.js | 17 +++++++---------- src/core/image_utils.js | 25 +++++++++---------------- 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/core/evaluator.js b/src/core/evaluator.js index 29120b2595a20..2f0fe58212512 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -795,7 +795,6 @@ class PartialEvaluator { patternDict, operatorList, task, - cacheKey, localTilingPatternCache ) { // Create an IR of the pattern code. @@ -825,8 +824,8 @@ class PartialEvaluator { operatorList.addDependencies(tilingOpList.dependencies); operatorList.addOp(fn, tilingPatternIR); - if (cacheKey) { - localTilingPatternCache.set(cacheKey, patternDict.objId, { + if (patternDict.objId) { + localTilingPatternCache.set(/* name = */ null, patternDict.objId, { operatorListIR, dict: patternDict, }); @@ -1356,9 +1355,11 @@ class PartialEvaluator { const patternName = args.pop(); // SCN/scn applies patterns along with normal colors if (patternName instanceof Name) { - const name = patternName.name; + const rawPattern = patterns.getRaw(patternName.name); - const localTilingPattern = localTilingPatternCache.getByName(name); + const localTilingPattern = + rawPattern instanceof Ref && + localTilingPatternCache.getByRef(rawPattern); if (localTilingPattern) { try { const color = cs.base ? cs.base.getRgb(args, 0) : null; @@ -1373,11 +1374,8 @@ class PartialEvaluator { // Handle any errors during normal TilingPattern parsing. } } - // TODO: Attempt to lookup cached TilingPatterns by reference as well, - // if and only if there are PDF documents where doing so would - // significantly improve performance. - const pattern = patterns.get(name); + const pattern = this.xref.fetchIfRef(rawPattern); if (pattern) { const dict = isStream(pattern) ? pattern.dict : pattern; const typeNum = dict.get("PatternType"); @@ -1392,7 +1390,6 @@ class PartialEvaluator { dict, operatorList, task, - /* cacheKey = */ name, localTilingPatternCache ); } else if (typeNum === PatternType.SHADING) { diff --git a/src/core/image_utils.js b/src/core/image_utils.js index b1a7e1d4eedde..abb6250f0f189 100644 --- a/src/core/image_utils.js +++ b/src/core/image_utils.js @@ -135,25 +135,18 @@ class LocalGStateCache extends BaseLocalCache { } class LocalTilingPatternCache extends BaseLocalCache { - set(name, ref = null, data) { - if (typeof name !== "string") { - throw new Error( - 'LocalTilingPatternCache.set - expected "name" argument.' - ); - } - if (ref) { - if (this._imageCache.has(ref)) { - return; - } - this._nameRefMap.set(name, ref); - this._imageCache.put(ref, data); - return; + constructor(options) { + super({ onlyRefs: true }); + } + + set(name = null, ref, data) { + if (!ref) { + throw new Error('LocalTilingPatternCache.set - expected "ref" argument.'); } - // name - if (this._imageMap.has(name)) { + if (this._imageCache.has(ref)) { return; } - this._imageMap.set(name, data); + this._imageCache.put(ref, data); } }