-
-
Notifications
You must be signed in to change notification settings - Fork 159
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
remove hasOwnProperty call #183
Conversation
@@ -121,7 +120,7 @@ export function parse( | |||
const key = str.slice(keyStartIdx, keyEndIdx); | |||
|
|||
// only assign once | |||
if (!__hasOwnProperty.call(obj, key)) { | |||
if (obj[key] === undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you double check performance? I didn’t change this because my numbers were worse doing the undefined check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, let me bench it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bench LGTM, would you be free to rebase on main and remove the if (value !== undefined)
now? Since it won't matter if dec
returns undefined
from #179.
On my Linux machine (CPU 12th Gen Intel(R) Core(TM) i9-12900H) New: ✓ src/parse.bench.ts (26) 39576ms
✓ parse (6) 7940ms
name hz min max mean p75 p99 p995 p999 rme samples
· simple 9,702,920.99 0.0001 0.2114 0.0001 0.0001 0.0001 0.0002 0.0003 ±0.25% 4851461 fastest
· decode 4,651,623.54 0.0002 0.0680 0.0002 0.0002 0.0002 0.0003 0.0008 ±0.12% 2325812
· unquote 9,406,398.95 0.0001 0.7161 0.0001 0.0001 0.0001 0.0002 0.0003 ±0.31% 4703200
· duplicates 1,929,250.28 0.0004 0.2137 0.0005 0.0005 0.0006 0.0007 0.0015 ±0.15% 964626
· 10 cookies 827,978.72 0.0010 0.4642 0.0012 0.0012 0.0019 0.0024 0.0081 ±0.67% 413990
· 100 cookies 66,803.29 0.0128 0.5871 0.0150 0.0146 0.0230 0.0285 0.2132 ±0.75% 33402 slowest
✓ parse top-sites (20) 31633ms
name hz min max mean p75 p99 p995 p999 rme samples
· parse accounts.google.com 7,581,630.32 0.0001 0.0834 0.0001 0.0001 0.0002 0.0002 0.0004 ±0.13% 3790816
· parse amazon.com 2,568,358.75 0.0003 0.5899 0.0004 0.0004 0.0005 0.0006 0.0011 ±0.59% 1284180
· parse apple.com 7,825,036.53 0.0001 1.4933 0.0001 0.0001 0.0002 0.0003 0.0013 ±0.63% 3912519
· parse cloudflare.com 7,231,698.58 0.0001 0.3224 0.0001 0.0001 0.0002 0.0003 0.0009 ±0.61% 3615850
· parse docs.google.com 7,349,305.38 0.0001 0.1290 0.0001 0.0001 0.0002 0.0002 0.0003 ±0.14% 3674653
· parse drive.google.com 7,018,752.67 0.0001 0.2117 0.0001 0.0001 0.0002 0.0002 0.0003 ±0.17% 3509377
· parse en.wikipedia.org 2,102,850.00 0.0004 0.0437 0.0005 0.0005 0.0005 0.0006 0.0010 ±0.07% 1051425
· parse istockphoto.com 536,774.79 0.0015 0.6531 0.0019 0.0019 0.0022 0.0025 0.0112 ±0.73% 268388 slowest
· parse maps.google.com 7,355,024.25 0.0001 4.1665 0.0001 0.0001 0.0002 0.0002 0.0006 ±1.77% 3677513
· parse play.google.com 7,561,780.41 0.0001 0.5561 0.0001 0.0001 0.0002 0.0002 0.0005 ±0.60% 3780891
· parse policies.google.com 7,608,549.95 0.0001 0.5061 0.0001 0.0001 0.0002 0.0002 0.0005 ±0.77% 3804275
· parse pt.wikipedia.org 2,670,298.36 0.0003 0.0561 0.0004 0.0004 0.0004 0.0005 0.0010 ±0.12% 1335150
· parse sites.google.com 7,172,833.14 0.0001 0.2123 0.0001 0.0001 0.0002 0.0002 0.0007 ±0.15% 3586417
· parse support.google.com 4,458,951.07 0.0002 0.7200 0.0002 0.0002 0.0003 0.0003 0.0008 ±0.97% 2229476
· parse t.me 7,294,171.24 0.0001 0.9658 0.0001 0.0001 0.0002 0.0002 0.0003 ±0.42% 3647086
· parse vk.com 853,759.58 0.0010 0.3518 0.0012 0.0012 0.0013 0.0017 0.0041 ±0.52% 426964
· parse www.google.com 4,583,636.97 0.0002 0.5562 0.0002 0.0002 0.0003 0.0003 0.0007 ±0.55% 2291819
· parse youtu.be 1,877,702.66 0.0005 0.2178 0.0005 0.0005 0.0006 0.0006 0.0014 ±0.16% 938852
· parse youtube.com 1,901,106.15 0.0005 0.2301 0.0005 0.0005 0.0006 0.0006 0.0012 ±0.16% 950554
· parse example.com 22,561,632.11 0.0000 0.2071 0.0000 0.0000 0.0001 0.0001 0.0001 ±0.17% 11280817 fastest Old (master / 0f56c6e): ✓ src/parse.bench.ts (26) 40091ms
✓ parse (6) 7787ms
name hz min max mean p75 p99 p995 p999 rme samples
· simple 8,727,527.23 0.0001 1.0084 0.0001 0.0001 0.0001 0.0002 0.0003 ±0.57% 4363764 fastest
· decode 4,127,381.38 0.0002 0.0453 0.0002 0.0002 0.0003 0.0004 0.0005 ±0.11% 2063691
· unquote 8,228,980.06 0.0001 0.2382 0.0001 0.0001 0.0001 0.0002 0.0003 ±0.20% 4114491
· duplicates 2,007,016.13 0.0004 1.5755 0.0005 0.0005 0.0006 0.0007 0.0022 ±0.76% 1003509
· 10 cookies 868,530.59 0.0010 0.6681 0.0012 0.0011 0.0014 0.0017 0.0066 ±0.47% 434266
· 100 cookies 68,317.46 0.0124 1.4316 0.0146 0.0142 0.0251 0.0383 0.1045 ±0.68% 34159 slowest
✓ parse top-sites (20) 32303ms
name hz min max mean p75 p99 p995 p999 rme samples
· parse accounts.google.com 7,279,393.53 0.0001 0.0678 0.0001 0.0001 0.0002 0.0002 0.0003 ±0.15% 3639697
· parse amazon.com 2,534,713.80 0.0003 0.3295 0.0004 0.0004 0.0005 0.0006 0.0011 ±0.40% 1267357
· parse apple.com 8,430,491.56 0.0001 0.0868 0.0001 0.0001 0.0002 0.0002 0.0003 ±0.13% 4215246
· parse cloudflare.com 8,000,964.86 0.0001 3.6636 0.0001 0.0001 0.0001 0.0002 0.0005 ±1.55% 4000483
· parse docs.google.com 7,647,191.59 0.0001 0.2127 0.0001 0.0001 0.0002 0.0002 0.0003 ±0.15% 3823596
· parse drive.google.com 7,698,992.83 0.0001 0.0532 0.0001 0.0001 0.0002 0.0002 0.0003 ±0.14% 3849497
· parse en.wikipedia.org 2,146,053.02 0.0004 0.5728 0.0005 0.0005 0.0006 0.0006 0.0010 ±0.35% 1073027
· parse istockphoto.com 550,595.79 0.0015 0.6311 0.0018 0.0018 0.0027 0.0030 0.0102 ±0.88% 275298 slowest
· parse maps.google.com 7,597,900.78 0.0001 5.0688 0.0001 0.0001 0.0002 0.0003 0.0006 ±2.12% 3798951
· parse play.google.com 8,055,075.47 0.0001 0.7307 0.0001 0.0001 0.0002 0.0002 0.0008 ±0.66% 4027538
· parse policies.google.com 7,681,858.95 0.0001 0.4727 0.0001 0.0001 0.0002 0.0002 0.0005 ±0.82% 3840933
· parse pt.wikipedia.org 2,568,763.73 0.0003 0.4443 0.0004 0.0004 0.0006 0.0007 0.0009 ±0.21% 1284382
· parse sites.google.com 7,427,749.51 0.0001 0.2212 0.0001 0.0001 0.0002 0.0002 0.0003 ±0.14% 3713875
· parse support.google.com 4,725,513.75 0.0002 0.6865 0.0002 0.0002 0.0003 0.0003 0.0008 ±1.12% 2362757
· parse t.me 7,036,034.16 0.0001 0.9734 0.0001 0.0001 0.0002 0.0002 0.0009 ±0.43% 3518018
· parse vk.com 900,729.42 0.0010 0.3716 0.0011 0.0011 0.0013 0.0017 0.0053 ±0.51% 450365
· parse www.google.com 4,584,557.85 0.0002 0.5046 0.0002 0.0002 0.0003 0.0003 0.0006 ±0.74% 2292279
· parse youtu.be 1,931,114.60 0.0004 0.2125 0.0005 0.0005 0.0006 0.0006 0.0016 ±0.17% 965558
· parse youtube.com 1,945,369.56 0.0005 0.2069 0.0005 0.0005 0.0006 0.0006 0.0014 ±0.14% 972685
· parse example.com 22,165,850.27 0.0000 0.2121 0.0000 0.0000 0.0001 0.0001 0.0001 ±0.19% 11082926 fastest |
I strongly recommend against this change. https://stackoverflow.com/a/46425266 & Please keep the __hasOwnProperty proper check which you introduced with version 0.7.2 Otherwise I will be forced to create another patch-package patch to revert this insecure change back to the secure code. Thank you. |
@NewEraCracker I think you misunderstand this change. It used to be an undefined check in 0.6 and this is also a null prototype now. |
@NewEraCracker the object in question doesn't have a prototype chain Line 98 in 0f56c6e
|
daeae56
to
44f6e63
Compare
Before:
After:
It still appears indifferent for me using node v22.9.0. I also found this site: https://andrew.hedges.name/experiments/in/. It seems like Edit: Either way I'm good with using |
I mean, in theory, The |
Since the object no longer has a prototype chain, we can just check if a key exists by accessing it