Is beartype cost acceptable?

[benoit74]

Context

While investigating a memory leak (fixed in #324 — a closure in RxRewriter.rewrite() was being re-decorated by beartype's claw hook on every call, permanently pinning it via beartype's own is_object_blacklisted cache), the fix removed the leak but also raised the question of how much runtime cost beartype_this_package() (used project-wide in zimscraperlib/__init__.py) actually adds. This issue is to share those numbers and discuss whether the tradeoff is the one we want.

Methodology

Ran warc2zim to completion, twice, on the same input, same machine, back to back:

warc2zim --publisher=openZIM --content-header-bytes-length=2048 \
  --zim-file=www.physicsclassroom.com_56e21a6e.zim --name=www.physicsclassroom.com_56e21a6e \
  --scraper-suffix "zimit 3.1.2" --output output \
  --url https://www.physicsclassroom.com/ \
  <local WARC, 731MB compressed, ~14.5K ZIM entries> --overwrite

wrapped in /usr/bin/time -v for wall-clock, CPU, and peak RSS.

• Run A: zimscraperlib as-is (beartype_this_package() active).
• Run B: same install, with the single line beartype_this_package() in zimscraperlib/__init__.py commented out (everything else identical), bytecode cache cleared between runs.

Both runs post-date the #324 leak fix, so memory is no longer confounded by that bug.

Results

	With beartype (Run A)	Without beartype (Run B)	Delta
Wall clock time	53:50 (3230s)	44:08 (2648s)	−18.0%
User CPU time	3835s	3251s	−15.2%
Max RSS	924,060 KB	885,356 KB	−4.2% (noise-level)

Both runs completed successfully with an identical entry count. Memory is essentially the same between the two (the small gap is within run-to-run noise).

Takeaway

Runtime type-checking via beartype_this_package() costs roughly 15-18% additional wall-clock/CPU time on this workload, with no meaningful memory cost of its own now that #324 is fixed.

Discussion

Is this tradeoff (broad runtime type-safety across zimscraperlib, for ~15-18% slower conversions) the one we want, or should beartype's coverage be scoped down — e.g. excluding hot paths like zimscraperlib.rewriting from beartype_this_package(), or using a cheaper BeartypeStrategy for those modules — while keeping full checking elsewhere, or allowing to fully disable beartype "on-demand" (e.g. enable it only for unit and e2e tests of scraperlib and scrapers)?

[rgaudin]

I'm not sure this test is representative. It's higher impact than I would have imagined. Good to know.

What's important in this discussion is what beartype provides: because scraperlib is a library and because Python is not a typed language, we have to assume any input at any place of the API and ensure nothing crazy happens. That's a lot of tedious work that we didn't want to do and the reason we're using beartype. We're also getting proper feedback errors at runtime which is nice.

beartype forced us to annotate types properly. If I recall correctly it was always meant to be temporary.

We have three choices going forward I think:

• keep beartype at runtime with its upside and downside.
• write/adapt input validation everywhere.
• inform our users that they have to respect the annotations or suffer the consequences and remove beatype. Assumption here is that developers uses type checking tool. We'd loose informative feedback on errors though.

My preference goes to the third one. We could still inject beartype in unit tests ; assuming we're covered 100% that would ensure we respect our end of the contract. We could document how one could plug it in as well so developers could still use it if they want to.

[benoit74]

What I had in mind is a sort of fourth choice: keep beartype by default but make it configurable (e.g. via an environment variable) so that few scraper Docker images can remove beartype.

I'm really not convinced we want to disable beartype on all our scrapers typically).

Novice users will benefit from beartype to avoid "stupid" bugs.

Scrapers automated tests will benefit from beartype as well.

When needed, scraper runtime will be able to disable beartype.

[rgaudin]

I understood but I'd consider it cleaner to let users (eg. scrapers) add it themselves. We could argue that the point of scraperlib is to do stuff that are meant to be duplicated in every scraper as well. In this case, I'd suggest we leave it on by default and only allow it to be disabled.

[benoit74]

We could argue that the point of scraperlib is to do stuff that are meant to be duplicated in every scraper as well. In this case, I'd suggest we leave it on by default and only allow it to be disabled.

This is what I meant. Provide sensible default behavior for all scraperlib user with a possibility to opt-out if one knows what he is doing (hopefully).