From ec9618100ed9b9d2e9b40ea34ce037ba1edf71db Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Thu, 20 Jul 2023 13:59:35 +0100 Subject: [PATCH 1/8] Cache repeated subtables while compiling colour fonts --- Lib/fontTools/colorLib/table_builder.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Lib/fontTools/colorLib/table_builder.py b/Lib/fontTools/colorLib/table_builder.py index f1e182c432..de46ba96dc 100644 --- a/Lib/fontTools/colorLib/table_builder.py +++ b/Lib/fontTools/colorLib/table_builder.py @@ -5,6 +5,8 @@ import collections import enum +import json +import copy from fontTools.ttLib.tables.otBase import ( BaseTable, FormatSwitchingBaseTable, @@ -84,6 +86,7 @@ def __init__(self, callbackTable=None): if callbackTable is None: callbackTable = {} self._callbackTable = callbackTable + self.cache = {} def _convert(self, dest, field, converter, value): enumClass = getattr(converter, "enumClass", None) @@ -124,6 +127,10 @@ def build(self, cls, source): if isinstance(source, cls): return source + key = json.dumps(source, sort_keys=True) + if key in self.cache: + return copy.deepcopy(self.cache[key]) + callbackKey = (cls,) fmt = None if issubclass(cls, FormatSwitchingBaseTable): @@ -178,6 +185,8 @@ def build(self, cls, source): (BuildCallback.AFTER_BUILD,) + callbackKey, lambda d: d )(dest) + self.cache[key] = dest + return dest From 32dff74047996f1df277e50cf78c49c8fbbb1c9a Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Thu, 20 Jul 2023 14:26:46 +0100 Subject: [PATCH 2/8] Skip things which can't be cached --- Lib/fontTools/colorLib/table_builder.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Lib/fontTools/colorLib/table_builder.py b/Lib/fontTools/colorLib/table_builder.py index de46ba96dc..510233403c 100644 --- a/Lib/fontTools/colorLib/table_builder.py +++ b/Lib/fontTools/colorLib/table_builder.py @@ -127,9 +127,14 @@ def build(self, cls, source): if isinstance(source, cls): return source - key = json.dumps(source, sort_keys=True) - if key in self.cache: - return copy.deepcopy(self.cache[key]) + cacheable = True + key = None + try: + key = json.dumps(source, sort_keys=True) + if key in self.cache: + return copy.deepcopy(self.cache[key]) + except TypeError: + cacheable = False callbackKey = (cls,) fmt = None @@ -185,7 +190,8 @@ def build(self, cls, source): (BuildCallback.AFTER_BUILD,) + callbackKey, lambda d: d )(dest) - self.cache[key] = dest + if cacheable: + self.cache[key] = dest return dest From 6ff8f34ddae77115f12e9ce797dfd5c8843f1888 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Fri, 21 Jul 2023 12:27:29 +0100 Subject: [PATCH 3/8] Store hash instead of storing full JSON --- Lib/fontTools/colorLib/table_builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/fontTools/colorLib/table_builder.py b/Lib/fontTools/colorLib/table_builder.py index 510233403c..874096e5c9 100644 --- a/Lib/fontTools/colorLib/table_builder.py +++ b/Lib/fontTools/colorLib/table_builder.py @@ -130,7 +130,7 @@ def build(self, cls, source): cacheable = True key = None try: - key = json.dumps(source, sort_keys=True) + key = hash(json.dumps(source, sort_keys=True)) if key in self.cache: return copy.deepcopy(self.cache[key]) except TypeError: From 235e89adbe59835fcc9071ae140e87299a69da7b Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 21 Jul 2023 13:39:06 +0100 Subject: [PATCH 4/8] colorLib: use sha256 hash and pickle instead of json for TableBuilder cache --- Lib/fontTools/colorLib/table_builder.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/Lib/fontTools/colorLib/table_builder.py b/Lib/fontTools/colorLib/table_builder.py index 874096e5c9..954d7bbe67 100644 --- a/Lib/fontTools/colorLib/table_builder.py +++ b/Lib/fontTools/colorLib/table_builder.py @@ -5,8 +5,9 @@ import collections import enum -import json import copy +import pickle +from hashlib import sha256 from fontTools.ttLib.tables.otBase import ( BaseTable, FormatSwitchingBaseTable, @@ -127,14 +128,9 @@ def build(self, cls, source): if isinstance(source, cls): return source - cacheable = True - key = None - try: - key = hash(json.dumps(source, sort_keys=True)) - if key in self.cache: - return copy.deepcopy(self.cache[key]) - except TypeError: - cacheable = False + key = sha256(pickle.dumps(source)).digest() + if key in self.cache: + return copy.deepcopy(self.cache[key]) callbackKey = (cls,) fmt = None @@ -190,8 +186,7 @@ def build(self, cls, source): (BuildCallback.AFTER_BUILD,) + callbackKey, lambda d: d )(dest) - if cacheable: - self.cache[key] = dest + self.cache[key] = dest return dest From bb1389eeafb0538b048db2d79a61ef83ec775311 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 21 Jul 2023 14:22:10 +0100 Subject: [PATCH 5/8] table_builder: use pickle+unpickle, faster than deepcopy --- Lib/fontTools/colorLib/table_builder.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/fontTools/colorLib/table_builder.py b/Lib/fontTools/colorLib/table_builder.py index 954d7bbe67..02fd4e990f 100644 --- a/Lib/fontTools/colorLib/table_builder.py +++ b/Lib/fontTools/colorLib/table_builder.py @@ -130,7 +130,8 @@ def build(self, cls, source): key = sha256(pickle.dumps(source)).digest() if key in self.cache: - return copy.deepcopy(self.cache[key]) + # pickle/unpickle is faster than copy.deepcopy + return pickle.loads(pickle.dumps(self.cache[key])) callbackKey = (cls,) fmt = None From 259e013933c64242fd68a242040c53bfa6ea4b69 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 21 Jul 2023 14:38:42 +0100 Subject: [PATCH 6/8] table_builder: cache key must include class as well as source otherwise we'd confuse two objects with distinct classes that happen to have the same 'source' --- Lib/fontTools/colorLib/table_builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/fontTools/colorLib/table_builder.py b/Lib/fontTools/colorLib/table_builder.py index 02fd4e990f..aca2322785 100644 --- a/Lib/fontTools/colorLib/table_builder.py +++ b/Lib/fontTools/colorLib/table_builder.py @@ -128,7 +128,7 @@ def build(self, cls, source): if isinstance(source, cls): return source - key = sha256(pickle.dumps(source)).digest() + key = (cls, sha256(pickle.dumps(source)).digest()) if key in self.cache: # pickle/unpickle is faster than copy.deepcopy return pickle.loads(pickle.dumps(self.cache[key])) From 9d044605a976d1cb901896c4a70e1941d1543e4b Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 21 Jul 2023 15:53:07 +0100 Subject: [PATCH 7/8] table_builder: use a LRU cache to avoid growing unbounded maybe to choose the default cache size we need more experiments with real-world fonts --- Lib/fontTools/colorLib/table_builder.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/Lib/fontTools/colorLib/table_builder.py b/Lib/fontTools/colorLib/table_builder.py index aca2322785..d365437548 100644 --- a/Lib/fontTools/colorLib/table_builder.py +++ b/Lib/fontTools/colorLib/table_builder.py @@ -83,11 +83,15 @@ class TableBuilder: based on otData info for the target class. See BuildCallbacks. """ - def __init__(self, callbackTable=None): + def __init__(self, callbackTable=None, cacheMaxSize=128): if callbackTable is None: callbackTable = {} self._callbackTable = callbackTable - self.cache = {} + # a LRU cache of (cls, sha256(pickle.dumps(source)).digest()) => dest. + self.cache = collections.OrderedDict() if cacheMaxSize > 0 else None + # cacheMaxSize of 0 means no cache; None means no limit. + # Not sure what a good default cache size is, 128 is just a random number... + self.cacheMaxSize = cacheMaxSize def _convert(self, dest, field, converter, value): enumClass = getattr(converter, "enumClass", None) @@ -128,10 +132,13 @@ def build(self, cls, source): if isinstance(source, cls): return source - key = (cls, sha256(pickle.dumps(source)).digest()) - if key in self.cache: - # pickle/unpickle is faster than copy.deepcopy - return pickle.loads(pickle.dumps(self.cache[key])) + if self.cache is not None: + cacheKey = (cls, sha256(pickle.dumps(source)).digest()) + if cacheKey in self.cache: + # Move to the end to mark it as most recently used + self.cache.move_to_end(cacheKey) + # return unique copies; pickle/unpickle is faster than copy.deepcopy + return pickle.loads(pickle.dumps(self.cache[cacheKey])) callbackKey = (cls,) fmt = None @@ -187,7 +194,11 @@ def build(self, cls, source): (BuildCallback.AFTER_BUILD,) + callbackKey, lambda d: d )(dest) - self.cache[key] = dest + if self.cache is not None: + if self.cacheMaxSize is not None and len(self.cache) >= self.cacheMaxSize: + # Evict the least recently used item at the front of the cache + self.cache.popitem(last=False) + self.cache[cacheKey] = dest return dest From 84c0617da87534108db096894f8a04213bd43393 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 24 Jul 2023 09:34:48 +0100 Subject: [PATCH 8/8] remove unused import copy --- Lib/fontTools/colorLib/table_builder.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/fontTools/colorLib/table_builder.py b/Lib/fontTools/colorLib/table_builder.py index d365437548..5fec97166d 100644 --- a/Lib/fontTools/colorLib/table_builder.py +++ b/Lib/fontTools/colorLib/table_builder.py @@ -5,7 +5,6 @@ import collections import enum -import copy import pickle from hashlib import sha256 from fontTools.ttLib.tables.otBase import (