Performance Counter, --with-perfctr

[jasnell]

The COUNTER_NET_SERVER_CONNECTION, COUNTER_NET_SERVER_CONNECTION_CLOSE, COUNTER_HTTP_SERVER_REQUEST, COUNTER_HTTP_SERVER_RESPONSE, COUNTER_HTTP_CLIENT_REQUEST, COUNTER_HTTP_CLIENT_RESPONSE functions are enabled only when the --with-perfctr option is enabled (which it always is on Windows).

These functions are enabled as globals in JavaScript and are only questionably useful. The Performance Counter support is undocumented in Node.js and it's really not clear if anyone is actually using that.

I'd like to deprecate these functions in 11 and remove them entirely in 12 and would look at using trace events instead as a channel for surfacing that kind of data.

Does anyone in @nodejs/diagnostics @nodejs/build @nodejs/performance know if these counters are for sure being used anywhere?

[jasnell]

My current plan is:

For Node.js 11:

• Convert the various COUNTER_* and DTRACE_* utility functions into an internalBinding with internal/* wrapper functions that will be non-op in JS if counters and dtrace are not available. This should result in a fairly sizable performance improvement.

• Change the current COUNTER_* and DTRACE_* globals into deprecated wrappers around the new internalBinding versions.

For Node.js 12:

• Remove the COUNTER_* and DTRACE_* globals (the internal bindings would still work)

• Begin refactoring the internal bindings for dtrace and counters to be more efficient

[jasnell]

See: nodejs/node#22485

[joaocgreis]

@jasnell sorry for the late reply, I wanted to give this some thought and review your PR.

Node Performance Counters have been broken since nodejs/node#16984 (would be good if someone else from @nodejs/platform-windows can confirm this so it's not only me, but I tried this on several VMs and reverting that PR fixes it). I believe it's safe to say that no one has used Performance Counters in 2018, and this includes all releases of v10.

Give the above, I suggest that we remove Performance Counters altogether.

To keep them, we'd need to revert the PR mentioned above and investigate the issue it reports. Performance Counters could probably also be improved, but given the lack of interest and the new trace events, it's probably better to avoid the maintenance burden.

[jasnell]

I'm good with removing them entirely

[jasnell]

Given this, my current plan is to remove the counter support entirely, with the current globals left in for now as deprecated non-ops.

[jasnell]

This has been resolved.