Repository navigation
Performance Counter, --with-perfctr #223
Description
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?
Activity
jasnell commentedon Aug 23, 2018
My current plan is:
For Node.js 11:
Convert the various
COUNTER_*andDTRACE_*utility functions into an internalBinding withinternal/*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_*andDTRACE_*globals into deprecated wrappers around the new internalBinding versions.For Node.js 12:
Remove the
COUNTER_*andDTRACE_*globals (the internal bindings would still work)Begin refactoring the internal bindings for dtrace and counters to be more efficient
jasnell commentedon Aug 23, 2018
See: nodejs/node#22485
joaocgreis commentedon Aug 31, 2018
@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 commentedon Aug 31, 2018
I'm good with removing them entirely
jasnell commentedon Sep 4, 2018
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 commentedon Nov 4, 2018
This has been resolved.