@joyeecheung build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1550/pipeline

Some local benchmark results:

                               confidence improvement accuracy (*)   (**)  (***)
 process/bench-env.js n=100000         **      1.99 %       ±1.39% ±1.84% ±2.40%

                    confidence improvement accuracy (*)   (**)  (***)
 os/cpus.js n=30000                -0.13 %       ±0.77% ±1.02% ±1.33%

                                        confidence improvement accuracy (*)   (**)  (***)
 http2/headers.js nheaders=0 n=1000                    0.33 %       ±1.06% ±1.40% ±1.83%
 http2/headers.js nheaders=10 n=1000                   0.44 %       ±1.03% ±1.37% ±1.79%
 http2/headers.js nheaders=100 n=1000                 -0.25 %       ±0.97% ±1.29% ±1.68%
 http2/headers.js nheaders=1000 n=1000         **      1.96 %       ±1.40% ±1.86% ±2.42%

The bench-parser results are rather flaky, and the benchmark is written in a strange way - it executes the same parser on the same payload in a hot loop, which almost never happen in the real world. When I turn the number of loop iteration down to 100 (probably the optimizing compiler doesn't kick in or doesn't do OSR doesn't optimize as aggressively that way) the improvement seems consistent, but I am not quite sure about that either. Then HTTP2 header parser that are migrated in the same way showed no significant impact, however, but that's benchmarked differently (it's not in a hot loop, instead it parse the headers sequentially in callbacks)

Intuitively the new API is doing what has to be done either in JS or C++ - it constructs an Array out of a FixedArray with all the elements readily packed inside, so it shouldn't be slower than the old way (which calls into JS), the impact seems to come from how the new approach affect what gets optimized in a hot loop which rarely happen in real word, so I am inclined to just ignore the results in http/bench-parser, but someone who has a better knowledge on the optimizing compiler may have a better insight than I am (cc @bmeurer ?).

For reference, something like #24125 shows significant improvement when the length of the array is long enough and the call cannot be optimized (e.g. unconditionally done in C++ land without kHasNoSideEffect notation)

From `node benchmark/compare.js --new ./node_new --old ./node_old --filter parser http`

                                       confidence improvement accuracy (*)   (**)   (***)
 http/bench-parser.js n=100000 len=16        ***    -13.15 %       ±7.09% ±9.44% ±12.29%
 http/bench-parser.js n=100000 len=32          *     -9.01 %       ±6.81% ±9.07% ±11.80%
 http/bench-parser.js n=100000 len=4         ***    -22.69 %       ±7.29% ±9.70% ±12.65%
 http/bench-parser.js n=100000 len=8         ***    -17.32 %       ±6.99% ±9.31% ±12.12%

Another time from `node benchmark/compare.js --new ./node_new --old ./node_old --filter parser http`

                                      confidence improvement accuracy (*)    (**)   (***)
 http/bench-parser.js n=100000 len=16               -12.83 %      ±17.71% ±23.56% ±30.67%
 http/bench-parser.js n=100000 len=32                -9.39 %      ±17.83% ±23.73% ±30.88%
 http/bench-parser.js n=100000 len=4           *    -21.00 %      ±17.30% ±23.04% ±30.01%
 http/bench-parser.js n=100000 len=8                -16.33 %      ±17.42% ±23.18% ±30.17%

Setting n=100:

                                   confidence improvement accuracy (*)   (**)  (***)
 http/bench-parser.js n=100 len=16        ***     27.89 %       ±2.98% ±3.98% ±5.19%
 http/bench-parser.js n=100 len=32        ***     40.82 %       ±2.65% ±3.56% ±4.70%
 http/bench-parser.js n=100 len=4           *      5.47 %       ±4.81% ±6.41% ±8.37%
 http/bench-parser.js n=100 len=8         ***     17.01 %       ±3.77% ±5.07% ±6.73%

Apparently using a fixed-size array in the http parser does the trick, though the impact shown from bench-parser.js is still influenced by the number of iterations, and the accuracy is still not too high when n=1e5 - I don't really know what to make out of this but I guess it doesn't really hurt ¯\(ツ)/¯

                                      confidence improvement accuracy (*)   (**)  (***)
 http/bench-parser.js n=100000 len=16        ***     10.44 %       ±3.90% ±5.20% ±6.76%
 http/bench-parser.js n=100000 len=32        ***     11.36 %       ±3.78% ±5.04% ±6.59%
 http/bench-parser.js n=100000 len=4           *      6.81 %       ±5.39% ±7.17% ±9.33%
 http/bench-parser.js n=100000 len=8         ***     11.29 %       ±3.82% ±5.09% ±6.63%

when n=100 (somehow accuracy is a little bit better):

                                   confidence improvement accuracy (*)   (**)  (***)
 http/bench-parser.js n=100 len=16        ***     47.65 %       ±2.68% ±3.57% ±4.65%
 http/bench-parser.js n=100 len=32        ***     62.44 %       ±3.34% ±4.46% ±5.85%
 http/bench-parser.js n=100 len=4         ***     21.83 %       ±4.10% ±5.47% ±7.17%
 http/bench-parser.js n=100 len=8         ***     41.69 %       ±3.60% ±4.83% ±6.36%

Other benchmark results:

                               confidence improvement accuracy (*)   (**)  (***)
 process/bench-env.js n=100000         **      3.65 %       ±2.36% ±3.14% ±4.09%

                    confidence improvement accuracy (*)   (**)  (***)
 os/cpus.js n=30000                -0.27 %       ±2.07% ±2.76% ±3.59%

                                       confidence improvement accuracy (*)   (**)  (***)
 http2/headers.js nheaders=0 n=1000                   -0.42 %       ±2.30% ±3.06% ±3.99%
 http2/headers.js nheaders=10 n=1000                   0.89 %       ±2.10% ±2.80% ±3.65%
 http2/headers.js nheaders=100 n=1000                  1.00 %       ±2.16% ±2.87% ±3.74%
 http2/headers.js nheaders=1000 n=1000          *      1.61 %       ±1.55% ±2.07% ±2.69%

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

Interesting results: I tried using reserve and then push_back or emplace_back in CreateHeaders (the hottest path in this patch), and they both cause a slight performance regression compared to assigning and indexing into the vector with high accuracy.

push_back:

                                      confidence improvement accuracy (*)   (**)  (***)
 http/bench-parser.js n=100000 len=16         **     -2.38 %       ±1.57% ±2.10% ±2.74%
 http/bench-parser.js n=100000 len=32        ***     -3.26 %       ±1.55% ±2.07% ±2.69%
 http/bench-parser.js n=100000 len=4         ***     -6.79 %       ±1.71% ±2.28% ±2.98%
 http/bench-parser.js n=100000 len=8         ***     -3.68 %       ±2.09% ±2.78% ±3.63%

emplace_back:

                                      confidence improvement accuracy (*)   (**)  (***)
 http/bench-parser.js n=100000 len=16                -0.92 %       ±1.81% ±2.41% ±3.13%
 http/bench-parser.js n=100000 len=32        ***     -5.43 %       ±2.19% ±2.92% ±3.80%
 http/bench-parser.js n=100000 len=4         ***     -4.89 %       ±1.88% ±2.52% ±3.29%
 http/bench-parser.js n=100000 len=8         ***     -2.76 %       ±1.58% ±2.11% ±2.75%

I suspect RVO is somehow not in effect with the way v8::Local<> is implemented/templated? Note that v8::Local<>'s default constructor merely initializes a pointer to 0 and the copy constructor doesn't do much either. But I prefer indexing into the vector with self-documenting equations anyway, so I didn't look into that any further.

Fixed a few nits with const references and a few comments. CI: https://ci.nodejs.org/job/node-test-pull-request/18576/

@joyeecheung can/should this be backported to v10.x?