doc: warn about using util.inspect/util.format in prod

[jasnell]

Because of the potential performance bottlenecks that may be introduced
by util.inspect() and util.format() in production code.

Based on real user feedback, it is not obvious that these are intended
to be debugging tools.

/cc @mcollina @lucamaraschi

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

doc

[Trott]

Two questions: Isn't util.format() used by console.log() and friends under the hood? Should there be a warning on those too?

Second, and I know this is an unpopular opinion, but can we remove *Note*: here? Everyone who adds a paragraph to our docs thinks it needs to be emphasized and that the way to do that is to use *Note*:. They're wrong on both counts. The way we render these, contrary to adding emphasis, it actually makes the paragraph look like an unimportant footnote.

These are non-blocking questions. I don't intend to demand an answer before this lands or anything. Just trying to offer some helpful suggestions.

[jasnell]

I think the console.*** methods are a bit more well understood to be debug tools and do not really need the warning. What I have seen in several cases, however, are folks using util.inspect() in logging code within hot paths, and that's really the kind of thing the warning is meant to discourage.

[lucamaraschi]

LGTM but I would probably make the warning for console.*** explicit too.

[BridgeAR]

I am confused by these comments. I recently optimized both functions in e.g. #14881 #14492 #15422 and when looking at the history, others optimized util.format as well.

Right now there is likely little that can be done to further improve the performance of those functions.

It should also not be much of a performance issue depending on the user input. If a huge array is inspected, it is indeed a lot of work. The problem in that case is that Object.keys has to be called in such a case and that is expensive. We could add a option to deactivate this default behavior for arrays, by the way, as it is only necessary for untypical arrays (extra properties on the array instead of only regular indices).

[jasnell]

Yes, they've been optimized about as much as they can be but they are still something that should never be used in hot paths. That's not what they've been optimized for and not what they are intended for. I've seen this pop up in flame graphs as a significant bottleneck a number of times (as recently as this morning in fact, which is what prompted this PR now).

[BridgeAR]

Well, using it in a hot code path is not really smart (and I wonder what they did - but that is another story). But the wording oft this should be different out of my perspective.

What about e.g.

Please note that `util.format()` is a synchronous method that is mainly intended
as debugging tool. Depending on the values to format it could have a significant
performance overhead that would block the event loop. Use this function
therefore with care and never in a hot code path.

[BridgeAR]

s/immportant/important

(also below)

[jasnell]

@BridgeAR ... PTAL now :-)

[BridgeAR]

👍

[mcollina]

LGTM but it needs to be rebased already.

[jasnell]

Rebased :-)

[jasnell]

Landed in a4f44ac

[MylesBorins]

This does not land cleanly on v9.x, would someone be willing to manually backport?

[targos]

Trivial conflict. Landed in 742b304.