Skip to content

Improve performance of denylist patterns#780

Merged
westonruter merged 11 commits intodevelopfrom
fix/denylist-pattern-performance
May 9, 2022
Merged

Improve performance of denylist patterns#780
westonruter merged 11 commits intodevelopfrom
fix/denylist-pattern-performance

Conversation

@westonruter
Copy link
Collaborator

@westonruter westonruter commented May 6, 2022

This fixes an issue reported in a support topic where URLs with many query parameters can result in very slow response times by the service worker. Hat tip to @jeffposnick for pointing this out in GoogleChrome/workbox#3077 (comment).

The regular expressions in question are:

  • /\?(.+&)*wp_customize=/
  • /\?(.+&)*customize_changeset_uuid=/

Given this example URL: https://example.com/?ab=4&v0=56&u0=degrees+Celsius+%28%C2%B0C%29&f0=C&v1=60&u1=degrees+Celsius+%28%C2%B0C%29&f1=C&v2=0.0005&u2=per+degree+Celsius+%28%2F%C2%B0C%29&f2=CdT&v3=100&u3=centimetres+%28cm%29&f3=cm&u4=centimetres+%28cm%29&f4=cm&v5=100&u5=sq+centimetres+%28cm%C2%B2%29&f5=sqcm&v6=100.4&u6=sq+centimetres+%28cm%C2%B2%29&f6=sqcm&v7=100&u7=cu+centimetres+%28cc%2C+cm%C2%B3%29&f7=cucm&v8=100.6&u8=cu+centimetres+%28cc%2C+cm%C2%B3%29&f8=cucm

The patterns exhibit catastrophic backtracking:

image

This PR updates the patterns to be more efficient.

Old Pattern Time
^\/wp\-admin($|\?.*|\/.*) 0.2ms
[^\?]*.\.php($|\?.*) 0.7ms
.*\?(.*&)?(wp_service_worker)= 1.1ms
.*\/wp\.serviceworker(\?.*)?$ 0.8ms
[^\?]*\/feed\/(\w+\/)?$ 1.0ms
\?(.+&)*wp_customize= >250ms
\?(.+&)*customize_changeset_uuid= >250ms
^\/wp\-json\/.* 0.1ms
New Pattern Time
^\/wp\-admin($|\?|\/) 0.0ms
^[^\?]*?\.php($|\?) 0.1ms
\?(.*?&)?wp_service_worker= 0.1ms
^[^\?]*?\/wp\.serviceworker(\?|$) 0.1ms
^[^\?]*?\/feed\/(\w+\/)?$ 0.1ms
\?(.*?&)?wp_customize= 0.1ms
\?(.*?&)?customize_changeset_uuid= 0.1ms
^\/wp\-json\/ 0.0ms
@westonruter westonruter added bug Something isn't working service-workers labels May 6, 2022
@westonruter westonruter requested a review from thelovekesh May 6, 2022 21:13
@codecov-commenter
Copy link

codecov-commenter commented May 6, 2022

Codecov Report

Merging #780 (5c063bb) into develop (50ff0cb) will increase coverage by 0.60%.
The diff coverage is 100.00%.

@@              Coverage Diff              @@
##             develop     #780      +/-   ##
=============================================
+ Coverage      19.01%   19.62%   +0.60%     
  Complexity       346      346              
=============================================
  Files             57       57              
  Lines           2324     2324              
=============================================
+ Hits             442      456      +14     
+ Misses          1882     1868      -14     
Flag Coverage Δ
php 19.62% <100.00%> (+0.60%) ⬆️
unit 19.62% <100.00%> (+0.60%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...wp-service-worker-navigation-routing-component.php 11.76% <100.00%> (+10.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50ff0cb...5c063bb. Read the comment docs.

@westonruter westonruter added this to the 0.7.1 milestone May 6, 2022
Copy link
Collaborator

@thelovekesh thelovekesh left a comment

Choose a reason for hiding this comment

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

Changes LGTM 👍🏼

@westonruter westonruter merged commit 74e5665 into develop May 9, 2022
@westonruter westonruter deleted the fix/denylist-pattern-performance branch May 9, 2022 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working service-workers

3 participants