Benchmark combo

[elidoran]

See #185 and #187

This PR applies the benchmarking stuff first. Then moves the combo stuff to the utils function with the original implementation. Then replaces it with the new implementation.

Run benchmark with node benchmark/arraying.js.

There's a variety of ways to setup how to run benchmarking scripts as you add more. Could use npm scripts entry per each benchmark. Or, make an index file in there which accepts a subcommand for which one to run so there's only one npm run script. I leave those decisions up to you.

Adding the new implementation is last so you can peel it off from the PR if you'd like.

[ljharb]

Could 3f1fc4b add tests for utils.combine, in particular ensuring that a and b are not mutated? That way, if that behavior needs to change in the "new implementation" commit, that commit can also alter the tests - so that the scope of the changes is apparent.

[elidoran]

I think the best place to let everyone know an internal change occurred is the CHANGELOG.

It doesn't seem a good use of time to write a test for something which doesn't need to exist and then immediately remove that test for a new implementation.

Mutation is the correct operation to append a value to an internally created and used array. That's why the push() function exists. User's only care about receiving the array for that param after parsing. It doesn't matter to them what the libary did to produce that array.

Ultimately, it's your library. Feel free to test how you like and accept, revise, or reject my PR's because the library's functionality is your responsibility.

I recommend you take the benchmarking out for a spin. Adapt it to compare current implementations against dev versions and try out some new stuff.

For example, you may consider altering parseObject() to build up the object from the tail of the array instead so it pop()'s off the end instead of shifting the array. Or, check out the use of concat() in stringify where push() is more appropriate.

It's an adventure. Have fun.

[ljharb]

I'm suggesting that this lets me, the reviewer, know what changes the PR is making - and the changelog is written by manually auditing the PR.

The other purpose of writing the tests is that it lets me merge in the first commit immediately, while it will take much more time for me to evaluate the rest of the PR. If you're not willing to take the time, that's entirely up to you.

Querystring parsing is not a use case that requires high performance, because there's only one query string per request, so there's no urgency to accept a performance improvement - especially if it changes observable semantics by adding mutation to a code path (which is far worse a thing than "slow")

Thanks for your contribution so far; I'll leave this open in case you decide to put in the time in the future.

[ljharb]

@elidoran I've improved the tests and pulled the combine util into master directly, and rebased this branch. Tests are now failing on this PR.

If you're still interested in seeing the performance of combine improve, I'd love to see you update this PR to get the tests passing.