Skip to content

Conversation

teimurjan
Copy link

Description

This PR replaces pixelmatch with blazediff in the E2E testing infrastructure to improve image comparison performance.

BlazeDiff is ~1.5x times faster than pixelmatch, producing the same results in type-safe code that is available in ESM and CJS.

BlazeDiff is already integrated in:

@teimurjan teimurjan requested review from a team and vtomole as code owners October 2, 2025 09:15
@teimurjan teimurjan requested a review from maffoo October 2, 2025 09:15
Copy link

google-cla bot commented Oct 2, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added the size: S 10< lines changed <50 label Oct 2, 2025
Copy link

codecov bot commented Oct 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.38%. Comparing base (704913b) to head (6ede0ce).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@ ## main #7690 +/- ## ========================================== - Coverage 99.38% 99.38% -0.01%  ========================================== Files 1089 1089 Lines 97550 97550 ========================================== - Hits 96949 96948 -1  - Misses 601 602 +1 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
@mhucka
Copy link
Contributor

mhucka commented Oct 13, 2025

@seunomonije since you've been working on cirq-web lately, would you be able to weigh in on this PR?

@seunomonije
Copy link
Contributor

seunomonije commented Oct 15, 2025

It seems to me that the proposed benefits of blazediff versus pixelmatch for cirq-web are:

  1. Faster image diff performance (the actual benchmarks can be found here). However, of the 32 benchmarks, 6 of them have a performance savings of >50ms, and 8 have a performance savings of >20ms.
  2. One less dependency, @types/pixelmatch, since blazediff has native TS support.

These benchmarks are webpages, landscape images, and images from the pixelmatch repo. It seems as if the biggest speed increases are dealing with bigger image sizes (>9MB), which we are unlikely to hit in cirq-web, even if we were to add new widgets that handle much larger, fault tolerant circuits.

Pixelmatch is lightweight, robust, still being maintained, and has a large community using it. It is also focused solely on image diffs. Blazediff seems to be building an entire ecosystem around diff checking, and is maintained by just the author.

@teimurjan can you comment on any other benefits of blazediff that I may have missed? Your work seems great, however, the benefits to cirq-web aren't obvious, and moving away from a mature package might be a rough point when trying to integrate into bigger projects. Especially for a dev dependency. As an aside, why not try and integrate the identical speedup algo into pixelmatch itself?

@teimurjan teimurjan force-pushed the perf/use-blazediff-instead-of-pixelmatch branch from 928747c to 340b0ef Compare October 15, 2025 06:19
@teimurjan
Copy link
Author

Hey @seunomonije!

You're absolutely right about the trade-offs, but I’d like to add a bit more context:

  • @blazediff/core is also a small package: around 13.2 kB compared to pixelmatch’s 19.4 kB (unpacked). It doesn’t include pngjs or other external dependencies, which makes it safer and more resilient at a time when supply-chain attacks on NPM packages are becoming more common.
  • You’re right that some tests show only small gains, but the biggest and most consistent improvement is for identical images, which make up most CI snapshots when test suites are passing. In those runs, the speed-up is typically 80–90 %, noticeably cutting CI and developer iteration times even if “worst-case” diffs perform similarly.

As for why I didn’t upstream these improvements to pixelmatch, I’m intentionally taking a different direction:

  • The I/O layer is fully decoupled from the core algorithm, keeping zero external dependencies and making it easy to plug in faster pipelines (e.g. sharp) without touching the diff logic.
  • BlazeDiff is meant to be a cohesive toolkit: image diffing, test matchers, and other data-type diffs under one roof. For instance, I already ship a deep object diff that outperforms microdiff. Keeping these tools consistent in design and performance makes long-term maintenance and DX much smoother.
@mhucka mhucka added triage/discuss Needs decision / discussion, bring these up during Cirq Cynque area/dependencies area/performance interface/cirq-web labels Oct 15, 2025
@pavoljuhas
Copy link
Collaborator

In our case the image comparison needs are tiny as cirq-web has only 3 images to compare.
There is no noticeable difference in CI run times, I see 37 seconds on the main branch vs the same time at the tip of this PR.
In fact it takes a bit more time on my Debian box to install the dev dependencies with check/npm ci after this PR (11 vs 8 seconds).

pixelmatch is only used for testing, we do not deploy it with the cirq-web Python package so the typescript package size is not of concern. Also supply-chain attacks would be more likely noticed for pixelmatch with its larger community of contributors.

Thank you @teimurjan for contributing this, but at this time we see no clear benefit to make the change.

@pavoljuhas pavoljuhas closed this Oct 15, 2025
@github-project-automation github-project-automation bot moved this from Waiting to Resolved in Follow-up tracker Oct 15, 2025
@pavoljuhas pavoljuhas removed the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment