[PATCH 2/2] dataclass_frozen_for_testing: add and use for WalkItem and GetResult

[Rob Browning]

The dataclass docs say that "there is a tiny perforance penalty when
using frozen=True". Testing has shown that the overhead can be
notable, so add a dataclass_frozen_for_testing decorator to compat
that only sets the real dataclass's frozen=True when BUP_TEST_LEVEL is
in the environment.

Change WalkItem and GetResult to rely on it since they're handled in
bulk.

Signed-off-by: Rob Browning <r...@defaultvalue.org>
Tested-by: Rob Browning <r...@defaultvalue.org>
---
lib/bup/cmd/get.py | 4 ++--
lib/bup/compat.py | 23 +++++++++++++++++++++++
lib/bup/git.py | 4 ++--
3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/lib/bup/cmd/get.py b/lib/bup/cmd/get.py
index e4590a86..cf648121 100644
--- a/lib/bup/cmd/get.py
+++ b/lib/bup/cmd/get.py
@@ -11,7 +11,7 @@ import os, re, sys, textwrap, time

from bup import client, compat, git, hashsplit, vfs
from bup.commit import commit_message
-from bup.compat import dataclass, get_argvb
+from bup.compat import dataclass, dataclass_frozen_for_testing, get_argvb
from bup.git import MissingObject, get_cat_data, parse_commit, walk_object
from bup.helpers import \
(EXIT_FAILURE,
@@ -348,7 +348,7 @@ def get_random_item(hash, src_repo, dest_repo, ignore_missing):
dest_repo.just_write(item.oid, item.type, item.data)


-@dataclass(slots=True, frozen=True)
+@dataclass_frozen_for_testing(slots=True)
class GetResult:
oid: Optional[bytes] = None
tree: Optional[bytes] = None
diff --git a/lib/bup/compat.py b/lib/bup/compat.py
index 32e7b10c..7ace4f90 100644
--- a/lib/bup/compat.py
+++ b/lib/bup/compat.py
@@ -57,3 +57,26 @@ else:
def dataclass(*args, **kwargs):
del kwargs['slots']
return dataclasses.dataclass(*args, **kwargs)
+
+# "frozen for testing" is handled as a separate decorator because if
+# it's handled dynamically, say as a bespoke frozen setting like
+# frozen='testing' for our dataclass above, pylint (st least 3.3.4)
+# gets confused and starts issuing false positives for no-member in
+# some cases (e.g. field() fields).
+#
+# We have this because because while the docs claim there is only "a
+# tiny performance penalty" for frozen=True[1], it's currently
+# expensive. Try "drecurse | pv -l > /dev/null" on a large tree with
+# and without a frozen stat_result, with a warm cache.
+#
+# [1] https://docs.python.org/3/library/dataclasses.html#frozen-instances
+
+if b'BUP_TEST_LEVEL' not in environ:
+ # So pylint can still understand it as a dataclasses.dataclass
+ dataclass_frozen_for_testing = dataclass
+else:
+ def dataclass_frozen_for_testing(*args, **kwargs):
+ """Exactly like dataclasses.dataclass except that frozen=True."""
+ assert 'frozen' not in kwargs, kwargs
+ kwargs['frozen'] = True
+ return dataclass(*args, **kwargs)
diff --git a/lib/bup/git.py b/lib/bup/git.py
index 01918f2f..10ba638e 100644
--- a/lib/bup/git.py
+++ b/lib/bup/git.py
@@ -17,7 +17,7 @@ from typing import Literal, Optional, Union

from bup import _helpers, hashsplit, midx, bloom, xstat
from bup.commit import create_commit_blob, parse_commit
-from bup.compat import dataclass, environ
+from bup.compat import dataclass_frozen_for_testing, environ
from bup.helpers import (EXIT_FAILURE,
OBJECT_EXISTS,
ObjectLocation,
@@ -1490,7 +1490,7 @@ class MissingObject(KeyError):
KeyError.__init__(self, f'object {hexlify(oid)!r} is missing')


-@dataclass(slots=True, frozen=True)
+@dataclass_frozen_for_testing(slots=True)
class WalkItem:
oid: bytes
name: bytes
--
2.47.3