Only hash model_fields, not whole __dict__

[alexmojaki]

Change Summary

This deals with just the __hash__ part of #7444, because initially I was worried that it would be hard to fix without a significant hit to performance. With this timing code:

import timeit

setup = """
import pydantic

A = pydantic.create_model("A", __config__=dict(frozen=True), **{f'x{i}': (int, i) for i in range(100)})

a = A()
"""

stmt = """
hash(a)
"""

print(timeit.timeit(setup=setup, stmt=stmt, number=1000000))

the previous approach is slightly slower for 5 fields, but about 60-70% faster for 100 fields. So this PR may slow down some programs a little, but in the larger scheme of things it's hard to imagine it making a real difference since it can still hash a million times in about 2 seconds.

Closes #7785

Checklist

• The pull request title is a good summary of the changes - it will be used in the changelog
• Unit tests for the changes exist
• Tests pass on CI
• Documentation reflects the changes where applicable
• My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

[QuentinSoubeyranAqemia]

Have you tried other implementation to compare performance, say a dict comprehension? I'd be curious to see how various implementation compare

[alexmojaki]

This:

        def hash_func(self: Any) -> int:
            d = self.__dict__
            return hash(self.__class__) + hash(tuple(d[k] for k in cls.model_fields))

is about 2 to 3 times slower than the implementation in this PR. I'm actually surprised it's not worse. We always hear about how slow Python is and how deferring to C makes a huge difference, but I often get underwhelming results when doing so.

[QuentinSoubeyranAqemia]

Interesting comparison !
While you're at it, what about:

def hash_func(self: Any) -> int:
            d = {self.__dict__[k] for k in cls.model_fields}
            return hash(self.__class__) + hash(tuple(d.values()))

Sometimes dict-comprehension are very fast

[alexmojaki]

I'll leave you to try these yourself. Also, d is a set there.

[QuentinSoubeyranAqemia]

I ran your benchmark and plotted the time taken as a function of the number of fields, your implementation is definitely the fastest, and is faster than the current one for very small objects.

I tweaked the implementation to call hash() only once; IIRC it is better to pass a tuple of things to hash, rather than manually summing the hases, because hash() will truncate the return value of __hash__, which can cause additional collisions.

This makes a good starting point to fix __eq__ with good performances, too

Code for the benchmark

import gc
import operator
import timeit
from typing import Type

import matplotlib.pyplot as plt
import pydantic
import tqdm.auto as tqdm


def set_hash_func_default(cls):
    pass


def set_hash_func_operator(cls: Type[pydantic.BaseModel]):
    getter = operator.itemgetter(*cls.model_fields.keys()) if cls.model_fields else lambda _: None

    def hash_func(self) -> int:
        return hash((self.__class__, getter(self.__dict__)))

    cls.__hash__ = hash_func


def set_hash_func_tuple(cls: Type[pydantic.BaseModel]):
    def hash_func(self) -> int:
        d = self.__dict__
        return hash((self.__class__, tuple(d[k] for k in cls.model_fields)))

    cls.__hash__ = hash_func


def set_hash_func_comprehension(cls: Type[pydantic.BaseModel]):
    def hash_func(self) -> int:
        d = self.__dict__
        fields = {k: d[k] for k in cls.model_fields}
        return hash((self.__class__, tuple(fields.values())))

    cls.__hash__ = hash_func


setup = """
import pydantic

Model = pydantic.create_model(
    "Model",
    __config__=dict(frozen=True),
    **{f'x{i}': (int, i) for i in range(%(size)d)}
)
set_hash_func(Model)
instance = Model()
"""
statement = "hash(instance)"

sizes = list(range(100))
N_EXEC = 10_000
ys = {}
for set_hash_func in [
    set_hash_func_default,
    set_hash_func_operator,
    set_hash_func_tuple,
    set_hash_func_comprehension,
]:
    namespace = {"set_hash_func": set_hash_func}
    ys[set_hash_func.__name__] = [
        # Take the min of the repeats as recommended by timeit doc
        min(
            timeit.Timer(setup=setup % {"size": size}, stmt=statement, globals=namespace).repeat(
                repeat=5, number=N_EXEC
            )
        )
        / N_EXEC
        for size in tqdm.tqdm(sizes, desc=set_hash_func.__name__)
    ]
    gc.collect()

fig, ax = plt.subplots()

for name, times in ys.items():
    ax.plot(sizes, times, label=name)
ax.set_title("Performance benchmark for __hash__ implementations")
ax.set_xlabel("Number of fields in the pydantic model")
ax.set_ylabel("Average hash() time (s)")

plt.legend()
plt.tight_layout()
fig.savefig("results.png", dpi=300, facecolor="white", transparent=False)

[alexmojaki]

I think the self.__class__ part should just be removed. That would fix the main bug in #7785 about generic classes. The alternative fix would be to instead hash the same type object used in __eq__ but this would reduce hash collisions very rarely (the field values are much more likely to differ than the type) and would slow things down in most cases.

[QuentinSoubeyranAqemia]

That makes sense. I didn't see #7785 before. I would also be in favor of keeping __hash__ simple, so make making it a strict subset of __eq__ is simple.
As you say, adding the class name in the hash is unlikely to avoid collisions, it is rare IMHO that two classes have the exact same set of attributes and values.

[dmontagu]

Is this faster than just doing

        def hash_func(self: Any) -> int:
            return hash(self.__class__) + hash(tuple(self.__dict__.get(k) for k in cls.model_fields))

I'd prefer to avoid adding the operator import if possible (as we have generally been trying to reduce the number of imports pydantic triggers, which increase startup time), but if it's noticeably faster then maybe it makes sense. I think if you can't see a significant and repeatable difference, better to not use it.

Unless I've misunderstood and the getter thing is doing something different from my suggestion.

[dmontagu]

Doh I see there was a whole conversation about this and confirmation that it is significantly faster. 👍

[dmontagu]

I still think we should confirm with @samuelcolvin though given the point about wanting to reduce import times.

[QuentinSoubeyranAqemia]

I see you propose using .get() instead of []. Do you know whether it is safe to use itemgetter(), which doesn't have a default and thus will raise an error if the key is not found?

Said differently, are there cases where some keys from cls.model_fields are not present on the instance?

[alexmojaki]

Said differently, are there cases where some keys from cls.model_fields are not present on the instance?

I tried to find ways that this might happen, which led me to #7784. I couldn't think of anything else but I don't actually know pydantic that deeply so I could easily be missing something. But my guess is that if it's possible then it's enough of a red flag that raising an error here would actually be a good thing.

[QuentinSoubeyranAqemia]

Regarding pydantic load time, it seems pydantic already imports operator at load time (possibly transitively):
python -c 'import sys, pydantic; print("operator" in sys.modules.keys())'
prints True

It could be that sys itself depends on operator, since python -c 'import sy; print("operator" in sys.modules.keys())' also yields true. But the interpreter also loads some modules by default when it starts, so it's a bit hard to know

[QuentinSoubeyranAqemia]

I performed a timing benchmark, I can't measure any difference in importing operator and pydantic vs. importing only pydantic.

Time pydantic alone:

for i in {1..100}; do { time -p python -c "import pydantic"; } 2>> import_time.txt; done
python -c 'import pathlib; print(min(float(line.split(" ")[1]) for line in pathlib.Path("import_time.txt").read_text().split("\n") if line.startswith("real"
)))'

Time pydantic + opeator:

for i in {1..100}; do { time -p python -c "import pydantic, operator"; } 2>> import_time_operator.txt; done
python -c 'import pathlib; print(min(float(line.split(" ")[1]) for line in pathlib.Path("import_time_operator.txt").read_text().split("\n") if line.startswith("real")))'

[QuentinSoubeyranAqemia]

I did further benchmark in case we need to handle some keys in cls.model_fields not being reflected on the instance __dict__.
I updated the benchmark above to report performance normalized by the current implementation, and commented out the comprehensions which are very slow.

Interestingly, performance of the stdlib varies with python version: on python 3.8, only wrapping __dict__ in a collections.defaultdict if some keys are missing doesn't improve performance. But on python 3.12, it degrades performances quite a bit for large models, but improves it for small models.

I guess we could implement a switch based on the size of the __dict__ to unconditionnally wrap above a certain size.

Python 3.8 runs

No injection of additional value in __dict__

Injection of additional value in `__dict__` (e.g. by `functools.cached_property`)

Python 3.12 runs

No injection of additional value in __dict__

Injection of additional value in `__dict__` (e.g. by `functools.cached_property`)

Benchmark code

Benchmark code

import collections
import dataclasses
import gc
import operator
import sys
import timeit
from typing import Any, Callable, Dict, Set, Tuple, Type

import matplotlib.pyplot as plt
import numpy as np
import pydantic
import tqdm.auto as tqdm

PYTHON_VERSION = ".".join(map(str, sys.version_info))


def safe_itemgetter(*items) -> Callable[[Dict[str, Any]], Tuple[Any, ...]]:
    if not items:
        return lambda _: tuple()

    key_set = frozenset(items)
    getter = operator.itemgetter(*items)

    def safe_wrapper(d: Dict[str, Any]) -> Tuple[Any, ...]:
        return (
            getter(d) if d.keys() >= key_set else getter(collections.defaultdict(lambda: None, d))
        )

    return safe_wrapper


def noop(*args, **kwargs):
    pass


def set_hash_func_itemgetter(cls: Type[pydantic.BaseModel]):
    getter = operator.itemgetter(*cls.model_fields.keys()) if cls.model_fields else lambda _: None

    def hash_func(self) -> int:
        return hash(getter(self.__dict__))

    cls.__hash__ = hash_func


def set_hash_func_itemgetter_defaultdict(cls: Type[pydantic.BaseModel]):
    getter = operator.itemgetter(*cls.model_fields.keys()) if cls.model_fields else lambda _: None

    def hash_func(self) -> int:
        return hash(getter(collections.defaultdict(lambda: None, self.__dict__)))

    cls.__hash__ = hash_func


def set_hash_func_itemgetter_safe(cls: Type[pydantic.BaseModel]):
    getter = safe_itemgetter(*cls.model_fields.keys())

    def hash_func(self) -> int:
        return hash(getter(self.__dict__))

    cls.__hash__ = hash_func


def set_hash_func_tuple_comprehension(cls: Type[pydantic.BaseModel]):
    def hash_func(self) -> int:
        d = self.__dict__
        return hash(tuple(d[k] for k in cls.model_fields))

    cls.__hash__ = hash_func


def set_hash_func_get_comprehension(cls: Type[pydantic.BaseModel]):
    def hash_func(self) -> int:
        d = self.__dict__
        return hash(tuple(d.get(k) for k in cls.model_fields))

    cls.__hash__ = hash_func


def set_hash_func_dict_comprehension(cls: Type[pydantic.BaseModel]):
    def hash_func(self) -> int:
        d = self.__dict__
        fields = {k: d[k] for k in cls.model_fields}
        return hash(tuple(fields.values()))

    cls.__hash__ = hash_func


SETUP = """
import pydantic

Model = pydantic.create_model(
    "Model",
    __config__=dict(frozen=True),
    **{f'x{i}': (int, i) for i in range(%(size)d)}
)
set_hash_func(Model)
instance = Model()
# Mimic @functools.cached_property editing __dict__
# instance.__dict__["not_a_field"] = None
"""
STATEMENT = "hash(instance)"


def time_hash_func(
    set_hash_func,
    *,
    sizes,
    n_execution: int,
    n_repeat: int = 5,
    progress_bar: bool = True,
) -> np.ndarray:
    namespace = {"set_hash_func": set_hash_func}
    times = [
        # Take the min of the repeats as recommended by timeit doc
        min(
            timeit.Timer(
                setup=SETUP % {"size": size},
                stmt=STATEMENT,
                globals=namespace,
            ).repeat(repeat=n_repeat, number=n_execution)
        )
        / n_execution
        for size in tqdm.tqdm(sizes, desc=set_hash_func.__name__, disable=not progress_bar)
    ]
    gc.collect()
    return np.array(times, dtype=float)


N_EXEC = 10_000
fig, ax = plt.subplots()

sizes = list(range(100))
baseline = time_hash_func(noop, sizes=sizes, n_execution=N_EXEC)

# Plot the baseline
ax.plot(sizes, baseline / baseline, label="baseline")

for label, set_hash_func in {
    "itemgetter": set_hash_func_itemgetter,
    "itemgetter+defaultdict": set_hash_func_itemgetter_defaultdict,
    "itemgetter+safety": set_hash_func_itemgetter_safe,
    # Those have bad performances and are deactivated to speed-up experimentation
    # "tuple-comprehension": set_hash_func_tuple_comprehension,
    # "get-comprehension": set_hash_func_get_comprehension,
    # "dict-comprehension": set_hash_func_dict_comprehension,
}.items():
    times = time_hash_func(set_hash_func, sizes=sizes, n_execution=N_EXEC)
    ax.plot(sizes, times / baseline, label=label)

ax.set_title(f"__hash__ timing benchmark (python {PYTHON_VERSION})")
ax.set_xlabel("Number of fields in the pydantic model")
ax.set_ylabel("Average time relative to baseline")

plt.legend()
plt.tight_layout()
fig.savefig(
    f"hash-benchmark_nocache_python-{PYTHON_VERSION}.png",
    dpi=300,
    facecolor="white",
    transparent=False,
)

[alexmojaki]

Thanks @QuentinSoubeyranAqemia for all your analysis.

Regarding importing operator, in addition to the points above, there's already a from operator import attrgetter in _generate_schema.py.

[QuentinSoubeyranAqemia]

I tried the following code, inspired by dataclasses, to try to get an instance that is missing a field. It doesn't seems to work, so I hope that pydantic enforces all fields to be present on the instance?

class Model(pydantic.BaseModel, frozen=True):
    attr: int = pydantic.Field(init_var=False)

m = Model()  # ValidationError

[QuentinSoubeyranAqemia]

@alexmojaki I think we still need to remove __class__ from the hash in this PR? Might as well fix #7785 which you opened while we are modifying __hash__.

[alexmojaki]

@alexmojaki I think we still need to remove __class__ from the hash in this PR? Might as well fix #7785 which you opened while we are modifying __hash__.

I agree, but that issue is still in pending state. @dmontagu WDYT, can I remove hash(self.__class__) + in this PR? See #7786 (comment) which is a bit buried in the conversation now.

[QuentinSoubeyranAqemia]

Maybe we could add a test in tests/test_edge_cases.py ? Looks like a good place to test for that

[QuentinSoubeyranAqemia]

@alexmojaki Just FYI, #7825 revealed an edge-case where you can have an instance that is missing some fields. This impacts your use of operator.itemgetter (I used it as well and that's why the problem showed up)

[alexmojaki]

Thanks @QuentinSoubeyranAqemia. That means this PR should also fix another bug, since the current implementation assumes that the order of __dict__ is reliable:

from pydantic import BaseModel


class A(BaseModel, frozen=True):
    x: int
    y: int


a = A(x=1, y=2)
a2 = a.copy(exclude={"x"}).copy(update={"x": 1})
print(a.__dict__)   # {'x': 1, 'y': 2}
print(a2.__dict__)  # {'y': 2, 'x': 1}
assert a == a2
assert hash(a) != hash(a2)

I've pushed a fix with the fastest overall approach I could find, and I expect it can be reused for __eq__.

While I'm at it I removed the hash(self.__class__). However this causes the test test_hash_function_give_different_result_for_different_object to fail, which expects identical-looking models with different classes to have different hashes. Going back to when this was all added, I found this comment providing some motivation:

my problem here is that I imagine in the following code f and b would have the same hash and therefore be "equal" ... Which is definitely not what people would expected.

I think this is a mistake, and that it's OK for the hashes to match if only the classes differ.

[QuentinSoubeyranAqemia]

I'll leverage the FallbackDict in the __eq__ implementation, thanks!

Getting models with missing fields is very weird, it violates the Liskov substitution principle. Interestingly, the documented alternative to the deprecated BaseModel.copy() method raise a ValidationError when excluding required fields.

For completeness sake's, I ran the benchmarks again with your safety mecanism: it has negligible performance impact on python 3.8, and since python 3.11 got zero-cost exception handling, impact is non-existent in python 3.11+.
It is faster than using collections.defaultdict unconditionally, which is nice, too.

Benchmark code (clock to reveal)

import collections
import gc
import itertools
import operator
import sys
import textwrap
import timeit
from os import name
from typing import Any, Dict, Iterable, List, Optional, Sized, Type

import matplotlib.pyplot as plt
import numpy as np
import pydantic
import tqdm.auto as tqdm
from matplotlib import axes, figure

PYTHON_VERSION = ".".join(map(str, sys.version_info))


def noop(*args, **kwargs):
    pass


def set_hash_func_itemgetter(cls: Type[pydantic.BaseModel]):
    getter = operator.itemgetter(*cls.model_fields.keys()) if cls.model_fields else lambda _: None

    def hash_func(self) -> int:
        return hash(getter(self.__dict__))

    cls.__hash__ = hash_func


def set_hash_func_itemgetter_defaultdict(cls: Type[pydantic.BaseModel]):
    getter = operator.itemgetter(*cls.model_fields.keys()) if cls.model_fields else lambda _: None

    def hash_func(self) -> int:
        return hash(getter(collections.defaultdict(lambda: None, self.__dict__)))

    cls.__hash__ = hash_func


class FallbackDict:
    def __init__(self, inner):
        self.inner = inner

    def __getitem__(self, key):
        return self.inner.get(key)


def set_hash_func_itemgetter_safe(cls: Type[pydantic.BaseModel]):
    getter = operator.itemgetter(*cls.model_fields.keys()) if cls.model_fields else lambda _: 0

    def hash_func(self: Any) -> int:
        try:
            return hash(getter(self.__dict__))
        except KeyError:
            # In rare cases (such as when using the deprecated copy method), the __dict__ may not contain
            # all model fields, which is how we can get here.
            # getter(self.__dict__) is much faster than any 'safe' method that accounts for missing keys,
            # and wrapping it in a `try` doesn't slow things down much in the common case.
            return hash(getter(FallbackDict(self.__dict__)))  # type: ignore

    cls.__hash__ = hash_func


def set_hash_func_tuple_comprehension(cls: Type[pydantic.BaseModel]):
    def hash_func(self) -> int:
        d = self.__dict__
        return hash(tuple(d[k] for k in cls.model_fields))

    cls.__hash__ = hash_func


def set_hash_func_get_comprehension(cls: Type[pydantic.BaseModel]):
    def hash_func(self) -> int:
        d = self.__dict__
        return hash(tuple(d.get(k) for k in cls.model_fields))

    cls.__hash__ = hash_func


def set_hash_func_dict_comprehension(cls: Type[pydantic.BaseModel]):
    def hash_func(self) -> int:
        d = self.__dict__
        fields = {k: d[k] for k in cls.model_fields}
        return hash(tuple(fields.values()))

    cls.__hash__ = hash_func


class SizedIterable(Sized, Iterable):
    pass


def run_benchmark(
    title: str,
    setup: str,
    statement: str,
    n_execution: int = 10_000,
    n_repeat: int = 5,
    globals: Optional[Dict[str, Any]] = None,
    progress_bar: bool = True,
    params: Optional[Dict[str, SizedIterable]] = None,
) -> np.ndarray:
    namespace = globals or {}
    # fast-path
    if not params:
        length = 1
        packed_params = [()]
    else:
        length = len(next(iter(params.values())))
        # This iterator yields a tuple of (key, value) pairs
        # First, make a list of N iterator over (key, value), where the provided values are iterated
        param_pairs = [zip(itertools.repeat(name), value) for name, value in params.items()]
        # Then pack our individual parameter iterator into one
        packed_params = zip(*param_pairs)

    times = [
        # Take the min of the repeats as recommended by timeit doc
        min(
            timeit.Timer(
                setup=setup % dict(local_params),
                stmt=statement,
                globals=namespace,
            ).repeat(repeat=n_repeat, number=n_execution)
        )
        / n_execution
        for local_params in tqdm.tqdm(
            packed_params, desc=title, total=length, disable=not progress_bar
        )
    ]
    gc.collect()

    return np.asarray(times, dtype=float)


def run_hash_benchmark(
    title: str,
    set_hash_func,
    sizes: SizedIterable,
    mimic_cached_property: bool,
    n_execution: int = 10_000,
    n_repeat: int = 5,
) -> np.ndarray:
    setup = textwrap.dedent(
        f"""
        import pydantic

        Model = pydantic.create_model(
            "Model",
            __config__=dict(frozen=True),
            **{{f'x{{i}}': (int, i) for i in range(%(size)d)}}
        )
        set_hash_func(Model)
        instance = Model()
        # Mimic @functools.cached_property editing __dict__
        {"instance.__dict__['not_a_field'] = None" if mimic_cached_property else ''}
        """
    )
    statement = "hash(instance)"
    namespace = {"set_hash_func": set_hash_func}
    return run_benchmark(
        title=title,
        setup=setup,
        statement=statement,
        n_execution=n_execution,
        n_repeat=n_repeat,
        globals=namespace,
        params={"size": sizes},
    )


def plot_benchmark(
    title: str,
    sizes: List[int],
    mimic_cached_property: bool,
    ax: Optional[axes.Axes] = None,
):
    ax = ax or plt.gca()

    baseline = run_hash_benchmark(
        title=f'{title}, baseline',
        set_hash_func=noop,
        sizes=sizes,
        mimic_cached_property=mimic_cached_property,
    )
    ax.plot(sizes, baseline / baseline, label='baseline')
    for label, set_hash_func in {
        "itemgetter": set_hash_func_itemgetter,
        "itemgetter+defaultdict": set_hash_func_itemgetter_defaultdict,
        "itemgetter+safety": set_hash_func_itemgetter_safe,
        # Those have bad performances and are deactivated to speed-up experimentation
        # "tuple-comprehension": set_hash_func_tuple_comprehension,
        # "get-comprehension": set_hash_func_get_comprehension,
        # "dict-comprehension": set_hash_func_dict_comprehension,
    }.items():
        times = run_hash_benchmark(
            title=f'{title}, {label}',
            set_hash_func=set_hash_func,
            sizes=sizes,
            mimic_cached_property=mimic_cached_property,
        )
        ax.plot(sizes, times / baseline, label=label)

    ax.set_title(title)
    ax.set_xlabel('Number of pydantic fields')
    ax.set_ylabel('Average time relative to baseline')
    return ax


def plot_all_benchmarks(sizes: List[int]) -> figure.Figure:
    n_rows, n_cols = 1, 2
    fig, axes = plt.subplots(n_rows, n_cols, figsize=(n_cols * 6, n_rows * 4))

    for col, mimic_cached_property in enumerate([False, True]):
        plot_benchmark(
            title=f'{mimic_cached_property=}',
            sizes=sizes,
            mimic_cached_property=mimic_cached_property,
            ax=axes[col],
        )

    for ax in axes.ravel():
        ax.legend()
    fig.suptitle(f"__hash__ timing benchmark (python {PYTHON_VERSION})")
    return fig


if __name__ == "__main__":
    fig = plot_all_benchmarks(list(range(100)))
    plt.tight_layout()
    target = f"hash-benchmark_python-{PYTHON_VERSION}.png"
    fig.savefig(
        target,
        dpi=300,
        facecolor="white",
        transparent=False,
    )
    print(f"Wrote {target}", file=sys.stderr)

[dmontagu]

@alexmojaki regarding the removal of the class in the hash — I think it might make more sense for it to grab the generic origin and include that in the hash (same as is done by the equality method). That would at least keep different classes with the same data dicts having different hashes, presumably reducing collisions. But obviously that's already going to be rare so maybe it is better to not worry about it and let the equality check differentiate. (And I guess it will make the common case of having different data a bit faster anyway due to eliminating the hashing of the type?) I'm happy to defer to you.

[dmontagu]

(@alexmojaki I'll wait to actually merge until you give a reply to the last comment about how you want to proceed, but to be clear I'm okay merging this as is if you think that's best.)

[alexmojaki]

I think it'd be so weird to have a dict/set containing a large number of models of different types (already an oddity in normal code) with the same data that it's worth optimising the common case.

[sydney-runkle]

LGTM overall, but I'd prefer to wait and merge once we release 2.5.0 - I don't think it makes sense to squeeze in changes between 2.5.0b1 and now. Happy to merge this mid-next week when we get the next release out.

Thanks for your work on this @alexmojaki!