jshttp / cookie

HTTP server cookie parsing and serialization
MIT License
1.36k stars 159 forks source link

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

Closed kurtextrem closed 1 month ago

kurtextrem commented 2 years ago

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:

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) ```
blakeembrey commented 1 month ago

Tried rebasing on the work I was doing in https://github.com/jshttp/cookie/pull/170. The alternative in https://github.com/jshttp/cookie/pull/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.