http: switch on string values

[sethbrenith]

Long ago, V8 was much faster switching on string lengths than values. That is no longer the case, so we can simplify a couple of methods.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes (sort of, see detail below)
• tests and/or benchmarks are included
• commit message follows commit guidelines

Affected core subsystem(s)

• http

Test results

I ran the test suite and found the following errors on Windows, which were consistent with or without this change. A couple of other errors also showed up inconsistently. Two of them seem to be covered by 18251, but I didn't see any active issue for test-assert.

=== release test-assert === Path: parallel/test-assert assert.js:68 throw new errors.AssertionError(obj); ^ AssertionError [ERR_ASSERTION]: 'The expression evaluated to a falsy value:\r\n\r\n assert((() => \'string\')()\n // eslint-disable-next-line\n ===\n strictEqual 'The expression evaluated to a falsy value:\r\n\r\n assert((() => \'string\')()\r\n // eslint-disable-next-line\r\n = at Object.innerFn (D:\repos\node\test\common\index.js:762:16) at expectedException (assert.js:374:19) at Function.throws (assert.js:419:16) at Object.expectsError (D:\repos\node\test\common\index.js:784:12) at Object.<anonymous> (D:\repos\node\test\parallel\test-assert.js:842:8) at Module._compile (module.js:661:30) at Object.Module._extensions..js (module.js:672:10) at Module.load (module.js:578:32) at tryModuleLoad (module.js:518:12) at Function.Module._load (module.js:510:3) Command: D:\repos\node\Release\node.exe D:\repos\node\test\parallel\test-assert.js === release test-http2-ping-flood === Path: sequential/test-http2-ping-flood (node:23136) ExperimentalWarning: The http2 module is an experimental API. Command: D:\repos\node\Release\node.exe D:\repos\node\test\sequential\test-http2-ping-flood.js --- TIMEOUT --- === release test-http2-settings-flood === Path: sequential/test-http2-settings-flood (node:30612) ExperimentalWarning: The http2 module is an experimental API. Command: D:\repos\node\Release\node.exe D:\repos\node\test\sequential\test-http2-settings-flood.js --- TIMEOUT --- 

Benchmark results

>Release\node.exe benchmark\compare.js --old Release_old\node.exe --new Release\node.exe --filter set_header http | rscript benchmark\compare.R [00:05:25|% 100| 1/1 files | 60/60 runs | 7/7 configs]: Done improvement confidence p.value http\\set_header.js n=1000000 value="Connection" 22.61 % *** 5.803813e-14 http\\set_header.js n=1000000 value="Content-Length" 20.64 % *** 9.065010e-19 http\\set_header.js n=1000000 value="Content-Type" 23.51 % *** 6.291114e-26 http\\set_header.js n=1000000 value="Set-Cookie" 26.20 % *** 8.024882e-29 http\\set_header.js n=1000000 value="Transfer-Encoding" 15.54 % *** 6.562425e-14 http\\set_header.js n=1000000 value="Vary" 20.31 % *** 1.390219e-17 http\\set_header.js n=1000000 value="X-Powered-By" 21.68 % *** 7.662119e-17 

[gireeshpunathil]

CI: https://ci.nodejs.org/job/node-test-pull-request/12724/

[maclover7]

All CI failures on Windows are either fixed, or known flakey tests. Landing.

[maclover7]

Landed in aba6bc3

[MylesBorins]

Should this land on v6.x or v8.x? It lands cleanly on 8.x but will require a manual backport to 6

[sethbrenith]

I think probably not, because V8 performance characteristics have changed over time and we wouldn't want to regress old versions.