Skip to content

Conversation

@wfairclough
Copy link
Collaborator

@wfairclough wfairclough commented Feb 22, 2025

We detected a performance issue in our CI where the BinaryDelta process was taking over 4 hours to generate deltas between our macOS binaries. After timing individual bsdiff operations, I identified that a single diff was causing most of the slowdown. The issue occurred in the main macOS binary (Contents/MacOS/<AppName>) diffing process after product changes that removed approximately 22MB of binary code. By implementing patches from the ChromiumOS project's version of bsdiff, we reduced the entire BinaryDelta process from hours to just 5 minutes.

The changes introduce a mechanism to detect when bsdiff gets stuck processing large blocks of data that differ by less than 8 bytes. After 100 iterations in such a state, the algorithm breaks out of the loop to prevent performance degradation. Additionally, the search comparison operator was updated to include equality cases.

Original ChromiumOS Changes:

426e4aa AU: bsdiff: Expand pathological case where files differ by <8 bytes by Thieu Le · 13 years ago
58146f7 AU: Fix bsdiff hang by Thieu Le · 13 years ago
a055996 bsdiff: Speed up pathological case. by Thieu Le · 14 years ago
a055996
bsdiff: Speed up pathological case.

bsdiff does not properly handle the case where there is a large block of
data in the new file that only differs from the old file by less than 8
bytes.  This causes bsdiff to continue searching through the files one
byte at a time and at each byte, re-compare the same large block of data
which leads to excessively long run times.  This fix checks for this
edge condition and breaks out of the search loop early.  This retains
the size efficiency of the patch file for most binaries while preserving
the runtime efficiency for files that fall into this category.

https://chromium.googlesource.com/chromiumos/third_party/bsdiff/+/a055996c743add7a9558839276fd1e4994d16bd3%5E%21/#F0

58146f7
AU: Fix bsdiff hang

https://chromium.googlesource.com/chromiumos/third_party/bsdiff/+/58146f74abd6b1b69693943195f37f4ac6a6acef%5E%21/#F0

426e4aa
AU: bsdiff: Expand pathological case where files differ by <8 bytes

Modify bsdiff to better handle the case where files differ by <8 bytes
in some regions, not limitting this case to linear traversal.

BUG=chromium-os:28552
TEST=Manual bsdiff of problematic files, update engine unit tests

https://chromium.googlesource.com/chromiumos/third_party/bsdiff/+/426e4aa1cbeb3c8a73002047d7a796ca8e5e17d4%5E%21/#F0

Misc Checklist

  • My change requires a documentation update on Sparkle's website repository
  • My change requires changes to generate_appcast, generate_keys, or sign_update

Testing

I tested and verified my change by using one or multiple of these methods:

  • Sparkle Test App
  • Unit Tests
  • My own app
  • Other (please specify)

macOS version tested: 15.3

Copy link
Collaborator Author

@wfairclough wfairclough changed the title Port bsdiff optimizations for diffing large blocks that only differ by less than 8 bytes Add fuzzing detection to prevent bsdiff infinite loops Feb 22, 2025
@wfairclough wfairclough changed the title Add fuzzing detection to prevent bsdiff infinite loops Improves bsdiff performance by preventing excessive iterations when processing similar data blocks. Feb 22, 2025
@wfairclough wfairclough changed the title Improves bsdiff performance by preventing excessive iterations when processing similar data blocks. Improves bsdiff performance by preventing excessive iterations when processing similar data blocks Feb 22, 2025
@wfairclough wfairclough marked this pull request as ready for review February 22, 2025 20:23
@tristanlabelle
Copy link

Would upstream accept this PR so we don't have to fork?

@wfairclough wfairclough changed the title Improves bsdiff performance by preventing excessive iterations when processing similar data blocks Improves bsdiff performance by preventing excessive iterations when processing similar data blocks Feb 24, 2025
Copy link
Collaborator Author

@wfairclough wfairclough left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants