Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shift try/catch to decode #179

Merged
merged 2 commits into from
Oct 8, 2024
Merged

Shift try/catch to decode #179

merged 2 commits into from
Oct 8, 2024

Conversation

blakeembrey
Copy link
Member

Shifts the behavior of try/catch responsibility to allow for customization on the behavior of failing to decode a value. The current behavior was added in #5 where it seemed they wanted to error, but the commit decided to ignore.

Opening this for feedback since it doesn't seem helpful for users to return a value that wouldn't be valid.

Performance changes, after:

     name                   hz     min     max    mean     p75     p99    p995    p999     rme  samples
   · simple       9,249,725.56  0.0000  1.1636  0.0001  0.0001  0.0001  0.0002  0.0003  ±0.80%  4624863
   · decode       4,244,417.24  0.0001  0.0465  0.0002  0.0003  0.0003  0.0003  0.0004  ±0.10%  2122209
   · unquote      9,400,928.36  0.0000  0.2792  0.0001  0.0001  0.0001  0.0001  0.0003  ±0.65%  4700465   fastest
   · duplicates   1,954,356.87  0.0004  0.0553  0.0005  0.0005  0.0006  0.0006  0.0008  ±0.07%   977179
   · 10 cookies     733,505.31  0.0012  0.4945  0.0014  0.0013  0.0017  0.0019  0.0048  ±0.78%   366753
   · 100 cookies     65,145.48  0.0140  0.4051  0.0154  0.0150  0.0195  0.0250  0.2963  ±0.84%    32573   slowest
   ✓ parse top-sites (15) 23786ms
     name                                  hz     min     max    mean     p75     p99    p995    p999     rme   samples
   · parse accounts.google.com   7,932,294.60  0.0000  0.3187  0.0001  0.0001  0.0002  0.0002  0.0003  ±0.14%   3966148
   · parse apple.com             8,333,867.53  0.0000  0.8362  0.0001  0.0001  0.0001  0.0002  0.0003  ±1.62%   4166934
   · parse cloudflare.com        8,182,418.71  0.0000  0.3420  0.0001  0.0001  0.0001  0.0002  0.0004  ±0.69%   4091210
   · parse docs.google.com       8,013,928.56  0.0000  0.0500  0.0001  0.0001  0.0002  0.0002  0.0002  ±0.07%   4006965
   · parse drive.google.com      7,896,319.84  0.0000  0.0320  0.0001  0.0001  0.0002  0.0002  0.0003  ±0.07%   3948160
   · parse en.wikipedia.org      1,997,180.47  0.0004  0.4225  0.0005  0.0005  0.0006  0.0007  0.0007  ±0.29%    998591
   · parse linkedin.com          2,272,203.27  0.0003  0.2940  0.0004  0.0005  0.0005  0.0006  0.0013  ±0.33%   1136241
   · parse maps.google.com       4,837,423.90  0.0001  0.2633  0.0002  0.0002  0.0003  0.0003  0.0005  ±0.54%   2418712
   · parse microsoft.com         3,374,044.95  0.0002  0.2991  0.0003  0.0003  0.0003  0.0004  0.0007  ±0.43%   1687023
   · parse play.google.com       8,208,489.98  0.0000  0.3740  0.0001  0.0001  0.0002  0.0002  0.0004  ±0.73%   4104246
   · parse support.google.com    5,770,459.54  0.0001  0.0480  0.0002  0.0002  0.0002  0.0003  0.0004  ±0.09%   2885230
   · parse www.google.com        3,708,421.64  0.0001  0.4141  0.0003  0.0003  0.0003  0.0004  0.0008  ±1.09%   1854211
   · parse youtu.be              2,033,608.98  0.0004  0.0492  0.0005  0.0005  0.0005  0.0006  0.0010  ±0.09%   1016805
   · parse youtube.com           1,953,372.75  0.0004  0.3893  0.0005  0.0005  0.0007  0.0007  0.0013  ±0.19%    976687   slowest
   · parse example.com          21,364,390.80  0.0000  0.0285  0.0000  0.0000  0.0001  0.0001  0.0001  ±0.08%  10682196   fastest

Before:

     name                   hz     min      max    mean     p75     p99    p995    p999     rme  samples
   · simple       9,194,535.34  0.0000   1.3647  0.0001  0.0001  0.0002  0.0002  0.0004  ±1.13%  4597268   fastest
   · decode       4,253,821.21  0.0001   0.0507  0.0002  0.0003  0.0003  0.0003  0.0004  ±0.08%  2126911
   · unquote      8,557,109.64  0.0000   0.4991  0.0001  0.0001  0.0002  0.0002  0.0003  ±1.48%  4278555
   · duplicates   1,906,618.51  0.0004   0.0746  0.0005  0.0005  0.0007  0.0007  0.0015  ±0.13%   953310
   · 10 cookies     716,954.63  0.0012  11.1583  0.0014  0.0014  0.0016  0.0018  0.0062  ±4.39%   358478
   · 100 cookies     64,918.84  0.0138   1.9725  0.0154  0.0146  0.0341  0.0350  0.1295  ±0.91%    32460   slowest
   ✓ parse top-sites (15) 23848ms
     name                                  hz     min     max    mean     p75     p99    p995    p999     rme   samples
   · parse accounts.google.com   7,699,062.38  0.0000  1.1830  0.0001  0.0001  0.0002  0.0002  0.0003  ±0.48%   3849532
   · parse apple.com             7,537,076.40  0.0000  2.1975  0.0001  0.0001  0.0002  0.0003  0.0005  ±1.95%   3768539
   · parse cloudflare.com        7,495,582.76  0.0000  0.5151  0.0001  0.0001  0.0002  0.0002  0.0005  ±1.02%   3747792
   · parse docs.google.com       7,525,138.42  0.0000  2.1942  0.0001  0.0001  0.0002  0.0002  0.0003  ±0.93%   3762570
   · parse drive.google.com      7,607,241.41  0.0000  0.4654  0.0001  0.0001  0.0002  0.0002  0.0004  ±0.39%   3803621
   · parse en.wikipedia.org      2,007,783.43  0.0004  0.2854  0.0005  0.0005  0.0006  0.0006  0.0012  ±0.19%   1003892
   · parse linkedin.com          2,259,173.49  0.0003  0.0926  0.0004  0.0005  0.0006  0.0006  0.0013  ±0.14%   1129587
   · parse maps.google.com       4,500,795.05  0.0001  0.4909  0.0002  0.0002  0.0003  0.0003  0.0007  ±1.24%   2250398
   · parse microsoft.com         3,232,590.44  0.0002  0.5348  0.0003  0.0003  0.0004  0.0004  0.0010  ±0.87%   1616296
   · parse play.google.com       8,082,789.71  0.0000  0.3590  0.0001  0.0001  0.0002  0.0002  0.0005  ±0.67%   4041395
   · parse support.google.com    5,714,045.54  0.0001  0.0745  0.0002  0.0002  0.0002  0.0003  0.0004  ±0.10%   2857023
   · parse www.google.com        3,803,591.07  0.0001  0.4004  0.0003  0.0003  0.0003  0.0004  0.0007  ±0.97%   1901796
   · parse youtu.be              1,946,094.89  0.0004  0.5930  0.0005  0.0005  0.0007  0.0007  0.0012  ±0.35%    973048   slowest
   · parse youtube.com           1,998,340.54  0.0004  0.3523  0.0005  0.0005  0.0006  0.0006  0.0017  ±0.19%    999171
   · parse example.com          21,601,327.48  0.0000  0.0557  0.0000  0.0000  0.0001  0.0001  0.0001  ±0.10%  10800664   fastest

@blakeembrey
Copy link
Member Author

@gurgunday do you have any thoughts on usage in Fastify? Is there a valid use-case I'm missing where keeping the original string is helpful?

src/index.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

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

Agree with the removal of tryDecode

@blakeembrey blakeembrey marked this pull request as ready for review October 8, 2024 18:31
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (0ecf9bd) to head (ebc973f).
Report is 1 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #179   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines          162       160    -2     
  Branches        68        67    -1     
=========================================
- Hits           162       160    -2     

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

@blakeembrey blakeembrey merged commit 93a5b97 into master Oct 8, 2024
8 checks passed
@blakeembrey blakeembrey deleted the be/remove-quote-parse branch October 8, 2024 18:35
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.

4 participants