Added sync mode

[mcollina]

This is mainly to solve pinojs/pino#542, however I found out that it gives us a 20% speed bump when logging things with sync mode in pino benchmarks.

Benchmarks in pino with sonic-boom master:

$ node benchmarks/basic.bench.js
benchPino*10000: 162.391ms
benchPinoExtreme*10000: 108.874ms
benchPinoNodeStream*10000: 245.955ms

Benchmarks in pino with this PR, enabling sync mode for both destination and extreme:

$ node benchmarks/basic.bench.js
benchPino*10000: 147.822ms
benchPinoExtreme*10000: 83.608ms
benchPinoNodeStream*10000: 243.117ms

cc @jsumners @davidmarkclements

[davidmarkclements]

has the number dropped because Node's gotten faster?

[mcollina]

Yes! 🚀

I also adjusted the benchmark to be more realistic

[davidmarkclements]

is this just a case of optimizing for benchmarks though?

Does it show improvement in pino-http / hapi-pino benchmarks?

[mcollina]

is this just a case of optimizing for benchmarks though?

It seems so, yes. In fact, we are slowing things down with pino-http for example by 30% when using sync mode. It's still 20% faster than a standard Node.js stream.

Does it show improvement in pino-http / hapi-pino benchmarks?

Not at all.

---

TL;DR

I will assemble a PR to fix pinojs/pino#542 when using the standard destination. We will lose throughput, but I think that type of behavior should be opt-in. Extreme mode will stay as is, and user will be given an option.

[mcollina]

My plan for pino is to have:

1. sync mode if the destination is stdout, this solves Pino 5.8.1 fails to flush log on process exit pino#542.
2. retain extreme mode as is.
3. provide an option object to pino.destination() and pino.extreme(), so that users could in theory pass in sync: true or  sync: false. I plan to make the default for pino.destination() synchronous for safety reasons.

This should match the behavior of Node core as much as possible.

cc @jsumners @davidmarkclements

[davidmarkclements]

1. 👍
2. 👍
3. by definition, destination should be maximum safety, extreme should be maximum performance. From an API perspective I don't see much benefit to allowing sync true|false