Improve performance of hashRegExp lookup by ryanwilsonperkin · Pull Request #16759 · webpack/webpack

ryanwilsonperkin · 2023-02-24T21:20:38Z

Fixes #16758
For applications with a very large number of assets, the cost of invoking a single regular expression with many many values in a group becomes very high.

By changing to a list of regular expressions (with helper methods for maintaining the original design) we can get a large performance improvement.

A simple way to view the performance difference between these two approaches is with the following short scripts:

// benchmark-regex-group.js
const crypto = require('node:crypto');

function makeRandomString(length) {
  return crypto.randomBytes(length).toString('hex');
}

const name = makeRandomString(40);
const keys = Array.from(Array(3000), () => makeRandomString(20));
const regex = new RegExp(keys.join("|"), "g");

name.replace(regex, found => 'replaced');

const crypto = require('node:crypto');

function makeRandomString(length) {
  return crypto.randomBytes(length).toString('hex');
}

const name = makeRandomString(40);
const keys = Array.from(Array(3000), () => makeRandomString(20));
const regexes = keys.map(key => new RegExp(key, "g"));

let result = name;
for (const regex of regexes) {
  result = result.replace(regex, found => 'replaced');
}

Benchmarking these against each other I get the following results:

❯ hyperfine 'node benchmark-regex-many.js' 'node benchmark-regex-group.js'
Benchmark 1: node benchmark-regex-many.js
  Time (mean ± σ):      87.2 ms ± 143.6 ms    [User: 39.5 ms, System: 4.9 ms]
  Range (min … max):    39.3 ms … 495.8 ms    10 runs

Benchmark 2: node benchmark-regex-group.js
  Time (mean ± σ):     18.669 s ±  0.101 s    [User: 18.564 s, System: 0.054 s]
  Range (min … max):   18.556 s … 18.840 s    10 runs

Summary
  'node benchmark-regex-many.js' ran
  214.02 ± 352.29 times faster than 'node benchmark-regex-group.js'

What kind of change does this PR introduce?

Bugfix, performance

Did you add tests for your changes?

No, given that this is a performance improvement I'm not sure that the test suite is setup to test the difference.

Does this PR introduce a breaking change?

No, hashes remain the same

What needs to be documented once your changes are merged?
N/A

For applications with a very large number of assets, the cost of invoking a single regular expression with many many values in a group becomes very high. By changing to a list of regular expressions (with helper methods for maintaining the original design) we can get a large performance improvement.

webpack-bot · 2023-02-24T21:20:45Z

For maintainers only:

This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
This needs to be backported to webpack 4 (issue will be created when merged)

ryanwilsonperkin · 2023-02-25T00:27:36Z

One thing that I was unsure of was whether or not a given asset could be expected to have multiple asset hashes in it's filename. To be defensive against that case, I've designed hashReplace and hashMatch to always iterate through the entire array of regular expressions, but if there's a known invariant that only one can match then we can speed this up even more by breaking on the first match/replacement.

devvsakib

LGTM

TheLarkInn · 2023-03-03T19:52:34Z

@alexander-akait when you are back and available will you look at this?

lib/optimize/RealContentHashPlugin.js

TheLarkInn · 2023-03-08T15:45:04Z

Re-running failed jobs which look to be flakey and then auto-merge.

ryanwilsonperkin · 2023-03-08T16:47:37Z

@TheLarkInn looks like one last test still flaky: https://github.com/webpack/webpack/actions/runs/4266358978/jobs/7635349344

lvivski · 2023-03-10T02:42:21Z

@ryanwilsonperkin thanks for this PR! I was thinking of a similar change, while working on #16491, and was very excited to see you making it :)
Unfortunately, at least for our scenario (~2M LOC, 10k hashes to replace per runtime, 5+ runtimes, 5000 output js files) this change has degraded performance quite a bit. Previously our builds would spend around 600 seconds in RealContentHash, and 90s after #16491, now they timeout.

Even generating multiple RegExp's is slower than having a single huge one for us. Do you see the same improvements, you've mentioned in PR, after upgrading to the latest version? I'm trying to understand why it would be faster for your scenario, which sounds relatively similar to ours in terms of size and complexity, but much slower for us.

ryanwilsonperkin · 2023-03-10T03:09:29Z

Hey @lvivski thanks for bringing it to my attention. Could you include some information about the format of the hashes that you're replacing and the number and size of assets that you're replacing them in? And are you able to isolate the timeout to this part of the code?

lvivski · 2023-03-10T03:27:36Z

@ryanwilsonperkin

Could you include some information about the format of the hashes

we're using xxxhash64 8 chars.

the number and size of assets that you're replacing them in

~5 runtime assets
~10k hashes to replace. 5k from content hash and 5k through the hook with webpack-subresource-integrity. I've also tried disabling SRI and it didn't help.

And are you able to isolate the timeout to this part of the code?

Yes, after creating a custom webpack build with this change reverted, it works fine.

I have just ran your stress-test build from https://github.com/ryanwilsonperkin/webpack-16758

CPU: 2.4 GHz 8-Core Intel Core i9
RAM: 64 GB 2667 MHz DDR4
OS: Mac OS 13.2.1
NodeJS: v19.7.0

`v5.76.0`

Run 1: 50487 ms asset processing > RealContentHashPlugin
Run 2: 51002 ms asset processing > RealContentHashPlugin
Run 3: 51650 ms asset processing > RealContentHashPlugin

`v5.76.0 w/ changes from this PR reverted`

Run 1: 9812 ms asset processing > RealContentHashPlugin
Run 2: 10482 ms asset processing > RealContentHashPlugin
Run 3: 10761 ms asset processing > RealContentHashPlugin

Around 5x decrease in processing time if I revert this change.

webpack-bot added the PR: unreviewed label Feb 24, 2023

devvsakib approved these changes Feb 25, 2023

View reviewed changes

TheLarkInn assigned sokra and alexander-akait Mar 3, 2023

TheLarkInn added performance P3: Important labels Mar 3, 2023

alexander-akait reviewed Mar 7, 2023

View reviewed changes

lib/optimize/RealContentHashPlugin.js Show resolved Hide resolved

alexander-akait approved these changes Mar 7, 2023

View reviewed changes

webpack-bot added PR: reviewed-approved and removed PR: unreviewed labels Mar 7, 2023

TheLarkInn assigned TheLarkInn and unassigned sokra Mar 8, 2023

TheLarkInn self-requested a review March 8, 2023 15:42

TheLarkInn approved these changes Mar 8, 2023

View reviewed changes

lib/optimize/RealContentHashPlugin.js Show resolved Hide resolved

TheLarkInn enabled auto-merge March 8, 2023 15:44

ryanwilsonperkin mentioned this pull request Mar 8, 2023

Further improvements to performance of hash regex lookups in RealContentHashPlugin #16791

Closed

TheLarkInn disabled auto-merge March 8, 2023 17:16

TheLarkInn merged commit b84efe6 into webpack:main Mar 8, 2023

ryanwilsonperkin deleted the real-content-hash-regex-perf branch March 8, 2023 17:22

lasar mentioned this pull request Mar 9, 2023

Bump webpack from 5.75.0 to 5.76.0 lasar/dynalist-date-parser#46

Merged

msimmer mentioned this pull request Mar 15, 2023

chore(deps-dev): bump webpack from 5.74.0 to 5.76.0 triplecanopy/b-ber#449

Merged

This was referenced Mar 15, 2023

build(deps): bump webpack from 5.75.0 to 5.76.1 louiechristie/weather-winton#61

Merged

chore(deps): bump webpack from 5.75.0 to 5.76.1 louiechristie/newcrossities#28

Merged

kevin-s mentioned this pull request Mar 15, 2023

Bump webpack from 5.75.0 to 5.76.1 kevin-s/personal-portfolio#23

Merged

renovate bot mentioned this pull request Mar 16, 2023

chore(deps): update devdependencies ReoHakase/mystack#23

Merged

1 task

MilkaChucky mentioned this pull request Mar 18, 2023

Bump webpack and @angular-devkit/build-angular in /BookingWebApp MilkaChucky/BookingWebApp#74

Merged

renovate bot mentioned this pull request Jun 6, 2023

Update webpack family ykj-rails/rts-kit#70

Merged

1 task

renovate bot mentioned this pull request Jun 25, 2023

build(deps): update dependency webpack to v5 tjreigh/humans#92

Merged

1 task

This was referenced Jul 14, 2023

chore(deps): update dependency webpack to v5 mozilla-releng/shipit#883

Closed

Update dependency webpack to v5 jfrench206/audio-vibrator#59

Open

chore(deps): update dependency webpack to v5 - autoclosed abaicus/vue-simple-toast#34

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve performance of hashRegExp lookup#16759

Improve performance of hashRegExp lookup#16759
TheLarkInn merged 1 commit intowebpack:mainfrom
ryanwilsonperkin:real-content-hash-regex-perf

ryanwilsonperkin commented Feb 24, 2023 •

edited

Loading

Uh oh!

webpack-bot commented Feb 24, 2023

Uh oh!

ryanwilsonperkin commented Feb 25, 2023

Uh oh!

devvsakib left a comment

Uh oh!

TheLarkInn commented Mar 3, 2023

Uh oh!

Uh oh!

Uh oh!

TheLarkInn commented Mar 8, 2023

Uh oh!

ryanwilsonperkin commented Mar 8, 2023

Uh oh!

lvivski commented Mar 10, 2023 •

edited

Loading

Uh oh!

ryanwilsonperkin commented Mar 10, 2023

Uh oh!

lvivski commented Mar 10, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Conversation

ryanwilsonperkin commented Feb 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webpack-bot commented Feb 24, 2023

Uh oh!

ryanwilsonperkin commented Feb 25, 2023

Uh oh!

devvsakib left a comment

Choose a reason for hiding this comment

Uh oh!

TheLarkInn commented Mar 3, 2023

Uh oh!

Uh oh!

Uh oh!

TheLarkInn commented Mar 8, 2023

Uh oh!

ryanwilsonperkin commented Mar 8, 2023

Uh oh!

lvivski commented Mar 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanwilsonperkin commented Mar 10, 2023

Uh oh!

lvivski commented Mar 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

v5.76.0

v5.76.0 w/ changes from this PR reverted

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ryanwilsonperkin commented Feb 24, 2023 •

edited

Loading

lvivski commented Mar 10, 2023 •

edited

Loading

lvivski commented Mar 10, 2023 •

edited

Loading

`v5.76.0`

`v5.76.0 w/ changes from this PR reverted`