huggingface / huggingface/datatrove

xxhash is pinned to <4: the proper fix needs a decision on str-vs-bytes hashing

Open
#508 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
3.3k
Forks
302
Avg merge
2h 18m
Merged PRs (30d)
2

Description

`xxhash` is currently pinned to `<4` in the `processing` extra (#507). This issue records why, and why lifting the pin is not just a one-line change.

## What broke

[xxhash 4.0.0](https://pypi.org/project/xxhash/4.0.0/) (2026-08-12) removed implicit `str` encoding. `xxh32_intdigest` / `xxh64_intdigest` now raise `TypeError: Strings must be encoded before hashing` for `str` input.

`datatrove.utils.hashes.xxhash` passes `str` straight through, so with `xxhash>=4` every dedup pipeline fails: minhash, exact dedup, URL dedup, sentence dedup, bloom filter. Lockfiles hid this from CI; it surfaced in the `0.10.0rc1` release run, at the step that installs the built wheel and resolves dependencies fresh.

`datasets` also declares `xxhash` unbounded, but hashes bytes, so it is unaffected.

## Current state

`xxhash<4` in the `processing` extra, shipped in `0.10.0rc1`. This restores exactly the dependency set everything already had, with no behaviour change. It is a stopgap: an upper bound on a common dependency will eventually cause resolution conflicts for downstream users.

## Why the obvious fix needs thought

The obvious fix is to encode inside the wrapper:

```python
def xxhash64(data: str | bytes):
if isinstance(data, str):
data = data.encode("utf-8")
return xxhash.xxh64_intdigest(data)
```

UTF-8 is what xxhash `<4` encoded implicitly, so digests are unchanged and existing signatures stay comparable (verified against 4.0.0). But it costs measurably on the hottest call in the library — minhash hashes every shingle of every document. Micro-benchmark, 300k calls on a typical shingle:

| path | ns/call |
| --- | --- |
| xxhash 3.x, `str` in (current behaviour) | 72 |
| Python-side `.encode()` on every call | 102 |
| ... plus the `isinstance` guard | 122 |
| already-`bytes` input | 62 |

xxhash 3.x encoded in C using CPython's cached UTF-8 representation of the `str`. A Python-level encode allocates a new `bytes` object per call, which is why the fix is ~1.4x, and the guarded version ~1.7x. (Micro-benchmark only — I have not measured end-to-end impact on a real dedup run.)

The last row is the interesting one: hashing `bytes` is *faster* than what we do today. So the better fix may be to hash bytes end-to-end in the dedup steps rather than to encode inside the wrapper — turning a forced regression into a small win.

## A wrinkle for any bytes-based approach

`create_hash_func(config, input_type)` already dispatches str/bytes for sha1 (`sha1_hash64` vs `sha1_hash64_bytes`), but xxhash has a single pair that accepted both. The only caller passing a non-default `input_type` is `ExactDedup`, which derives it by reflection:

```python
hash_fc_type = inspect.signature(self.config.content_getter).return_annotation
```

If the user's `content_getter` is unannotated, or is defined in a module using `from __future__ import annotations` (where the annotation arrives as the string `"bytes"`, not the type), this does not resolve to `bytes` and the `str` function is returned. Today xxhash tolerates that mismatch because it accepted both types; a strict bytes/str split would surface it as a crash. Worth handling deliberately rather than discovering it downstream.

## Options

1. Encode in the wrapper, lift the pin. Smallest diff, ~1.4x on the hash call.
2. Hash bytes end-to-end in the dedup steps, lift the pin. Larger change, likely a net speedup, needs the reflection wrinkle above resolved.
3. Keep the pin for now. Zero risk, no user-visible cost today, revisit when something in the ecosystem actually needs `xxhash>=4`.

Option 1 is written and digest-verified if a quick unblock is ever wanted. Option 2 looks like the right long-term shape but deserves benchmarks on a real dedup run rather than a micro-benchmark.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at datatrove.utils.hashes.xxhash and create_hash_func, then inspect ExactDedup's content_getter annotation path and its str/bytes dispatch. Compare the listed approaches with an end-to-end dedup benchmark rather than only the micro-benchmark. Done means xxhash>=4 works without changing digest compatibility or breaking input-type handling.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
data-engineering, performance
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.