Skip to content

Shift try/catch to decode#179

Merged
blakeembrey merged 2 commits intomasterfrom
be/remove-quote-parse
Oct 8, 2024
Merged

Shift try/catch to decode#179
blakeembrey merged 2 commits intomasterfrom
be/remove-quote-parse

Conversation

@blakeembrey
Copy link
Member

@blakeembrey blakeembrey commented Oct 7, 2024

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

blakeembrey commented Oct 7, 2024

@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?

gurgunday
gurgunday reviewed Oct 8, 2024
View reviewed changes
gurgunday
gurgunday reviewed Oct 8, 2024
View reviewed changes
Copy link
Contributor

@gurgunday gurgunday left a comment •
edited
Loading

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

codecov-commenter commented Oct 8, 2024

⚠️ 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
@blakeembrey blakeembrey deleted the be/remove-quote-parse branch October 8, 2024 18:35
This was referenced Oct 8, 2024
Merged
Merged
@thetutlage thetutlage mentioned this pull request Nov 4, 2024
Closed
@DanielRivers DanielRivers mentioned this pull request Nov 6, 2024
Merged
@DunkeyMan-maker DunkeyMan-maker mentioned this pull request Nov 7, 2024
Open
@davidesigner davidesigner mentioned this pull request Dec 6, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@climba03003 climba03003 climba03003 left review comments

@gurgunday gurgunday gurgunday left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@blakeembrey @codecov-commenter @climba03003 @gurgunday

Comments