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

perf(parse): cache length, return early #144

Merged
merged 3 commits into from
Oct 2, 2024

Conversation

kurtextrem
Copy link
Contributor

This PR caches str.length (since the string length stays the same across the function).
In addition, I've also added an early return for cookie strings that are below the min. spec length and added a test for that.

Explanation

Previously, for e.g. empty strings as input we did the following things:

  • var opt = options || {};
  • var dec = opt.decode || decode
  • while (index < str.length)
  • var eqIdx = str.indexOf('=', index)
  • if (eqIdx === -1) break

now it is only if (length < 2) return, so it saves us time on early return.

do-while instead of while

Since we've taken care of the min string length, we can make the assumption there should always be one run of the statement.

less allocations

By reusing the vars, we produce less allocations and we don't allocate a {} in case options are missing.

benchmark

Before After
parse accounts.google.com x     5,157,003 ops/sec ±0.68% (190 runs sampled)
parse apple.com           x     5,631,643 ops/sec ±0.54% (189 runs sampled)
parse cloudflare.com      x     4,734,105 ops/sec ±0.66% (187 runs sampled)
parse docs.google.com     x     4,781,485 ops/sec ±0.47% (189 runs sampled)
parse drive.google.com    x     4,771,381 ops/sec ±0.59% (189 runs sampled)
parse en.wikipedia.org    x     1,594,865 ops/sec ±0.33% (191 runs sampled)
parse linkedin.com        x     1,039,303 ops/sec ±0.38% (191 runs sampled)
parse maps.google.com     x     2,458,153 ops/sec ±0.42% (191 runs sampled)
parse microsoft.com       x       317,199 ops/sec ±0.76% (194 runs sampled)
parse play.google.com     x     4,865,173 ops/sec ±0.53% (188 runs sampled)
parse plus.google.com     x     4,635,100 ops/sec ±0.46% (190 runs sampled)
parse sites.google.com    x     4,596,654 ops/sec ±0.49% (189 runs sampled)
parse support.google.com  x     2,954,600 ops/sec ±0.46% (190 runs sampled)
parse www.google.com      x     1,925,160 ops/sec ±0.37% (190 runs sampled)
parse youtu.be            x     1,919,612 ops/sec ±0.34% (192 runs sampled)
parse youtube.com         x     1,931,269 ops/sec ±0.26% (193 runs sampled)
parse example.com         x 1,282,216,677 ops/sec ±0.15% (196 runs sampled)

simple      x 5,903,507 ops/sec ±0.54% (189 runs sampled)
decode      x 1,216,111 ops/sec ±0.13% (196 runs sampled)
unquote     x 5,226,976 ops/sec ±0.46% (191 runs sampled)
duplicates  x 1,578,298 ops/sec ±0.38% (193 runs sampled)
10 cookies  x   477,935 ops/sec ±0.33% (192 runs sampled)
100 cookies x    41,227 ops/sec ±0.14% (195 runs sampled)
parse accounts.google.com x     5,251,799 ops/sec ±0.49% (189 runs sampled)
parse apple.com           x     5,831,906 ops/sec ±0.42% (191 runs sampled)
parse cloudflare.com      x     4,799,404 ops/sec ±0.68% (188 runs sampled)
parse docs.google.com     x     4,922,220 ops/sec ±0.48% (188 runs sampled)
parse drive.google.com    x     4,845,429 ops/sec ±0.53% (188 runs sampled)
parse en.wikipedia.org    x     1,579,713 ops/sec ±0.30% (192 runs sampled)
parse linkedin.com        x     1,023,814 ops/sec ±0.35% (191 runs sampled)
parse maps.google.com     x     2,443,190 ops/sec ±0.45% (190 runs sampled)
parse microsoft.com       x       314,187 ops/sec ±0.22% (195 runs sampled)
parse play.google.com     x     4,762,254 ops/sec ±0.43% (190 runs sampled)
parse plus.google.com     x     4,576,545 ops/sec ±0.53% (190 runs sampled)
parse sites.google.com    x     4,526,011 ops/sec ±0.72% (189 runs sampled)
parse support.google.com  x     2,925,142 ops/sec ±0.52% (191 runs sampled)
parse www.google.com      x     1,871,705 ops/sec ±0.40% (191 runs sampled)
parse youtu.be            x     1,929,021 ops/sec ±0.29% (193 runs sampled)
parse youtube.com         x     1,930,241 ops/sec ±0.40% (192 runs sampled)
parse example.com         x 1,294,237,122 ops/sec ±0.07% (195 runs sampled)

simple      x 6,020,155 ops/sec ±0.43% (191 runs sampled)
decode      x 1,198,982 ops/sec ±0.16% (194 runs sampled)
unquote     x 5,442,676 ops/sec ±0.84% (189 runs sampled)
duplicates  x 1,611,695 ops/sec ±0.46% (190 runs sampled)
10 cookies  x   507,256 ops/sec ±0.23% (193 runs sampled)
100 cookies x    41,737 ops/sec ±0.20% (194 runs sampled)

@kurtextrem kurtextrem changed the title perf(parse): remove usage of lastIndexOf perf(parse): cache length, return early Jul 14, 2022
@jshttp jshttp deleted a comment from gurgunday Jul 30, 2023
@blakeembrey
Copy link
Member

Tried rebasing on the work I was doing in #170. The alternative in #170 was to do a while (index < length - 2) check which would eliminate lines 4 & 5 from your list, as well as merges the new if with the while making it only 2/5. So at this point it's only eliminating the dec line, but I'll merge anyway, I'd love to see more perf wins!

Thanks for the PR.

@blakeembrey blakeembrey merged commit 9e7ca51 into jshttp:master Oct 2, 2024
3 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants