node icon indicating copy to clipboard operation
node copied to clipboard

src: add locksCounters() to monitor Web Locks usage

Open IlyasShabi opened this issue 1 month ago • 7 comments

Description

This PR introduces process.locksCounters() to provide real time visibility into Web Locks API usage and contention metrics across the Node.js process.

Motivation

Lock contention can be difficult to diagnose in concurrent applications. Other platforms already make this easier,.NET has lock performance counters, Java has JMX for checking thread locks, and Go has mutex profiling.

Node.js added the Web Locks API in v24, but there’s currently no built-in way to see how locks are behaving. The new process.locksCounters() provide metrics to help developers monitor and debug lock related performance problems.

IlyasShabi avatar Oct 30 '25 23:10 IlyasShabi

Review requested:

  • [ ] @nodejs/startup

nodejs-github-bot avatar Oct 30 '25 23:10 nodejs-github-bot

Codecov Report

:x: Patch coverage is 97.64706% with 2 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 88.57%. Comparing base (bdf03bf) to head (68a2665). :warning: Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/node_locks.cc 94.28% 1 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #60502      +/-   ##
==========================================
+ Coverage   88.05%   88.57%   +0.51%     
==========================================
  Files         704      704              
  Lines      207857   207941      +84     
  Branches    39964    40054      +90     
==========================================
+ Hits       183030   184182    +1152     
+ Misses      16806    15803    -1003     
+ Partials     8021     7956      -65     
Files with missing lines Coverage Δ
lib/internal/bootstrap/node.js 99.58% <100.00%> (+<0.01%) :arrow_up:
lib/internal/process/per_thread.js 99.47% <100.00%> (+2.36%) :arrow_up:
src/node_locks.h 91.48% <100.00%> (+3.61%) :arrow_up:
src/node_process_methods.cc 89.07% <100.00%> (+0.38%) :arrow_up:
src/node_locks.cc 74.39% <94.28%> (+1.92%) :arrow_up:

... and 102 files with indirect coverage changes

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Oct 31 '25 00:10 codecov[bot]

@Qard I put it on process because Locks are shared across the entire process and It follows the same pattern as memoryUsage() , cpuUsage() , and resourceUsage(). I can also move it to worker_threads if there are concerns about growing the process namespace.

IlyasShabi avatar Nov 02 '25 16:11 IlyasShabi

I think if we want to add a usage counter to process, it should ideally be generic, e.g. something like process.usageCounter('lock.abort'). We already have something similar in node_debug.h, just that so far these are limited to debug-only counters for internal tests.

Also, it would be nicer if the usage counter can be cleared/started/stopped at some given time instead of being recorded throughout the lifetime of the application.

joyeecheung avatar Nov 02 '25 17:11 joyeecheung

@joyeecheung I looked at the debug counters pattern, but I think it's better suited for internal testing rather than public API. I'm more with moving this to worker_threads as getLocksCounters() (happy to hear preferences on naming) and keep process namespace cleaner.

Also, it would be nicer if the usage counter can be cleared/started/stopped at some given time instead of being recorded throughout the lifetime of the application.

IMO I don't think they're a good fit for lock counters. These are lifetime counters, and adding clear function could creates problems: e.g. if library A clears the counters, library B loses visibility and can't get correct metrics.

Also, start/stop toggles make sense for heavy profiling (with stack traces), but for simple counter increments, the performance impact is negligible and could causes conflicts between libraries/workers

IlyasShabi avatar Nov 02 '25 19:11 IlyasShabi

@Qard @joyeecheung I would like to continue on this. Any thought how we can make a decision?

IlyasShabi avatar Nov 18 '25 10:11 IlyasShabi

IMO I don't think they're a good fit for lock counters. These are lifetime counters, and adding clear function could creates problems: e.g. if library A clears the counters, library B loses visibility and can't get correct metrics.

I meant that it would be nice to have individual counters that can be independently started/paused/stopped, so that e.g. you can use it in tests, as opposed to only having one global counter. For example, if one library A does diagnostics and another library B uses locks in some way, A can start its own counter to monitor code that depends on B, B won't be able to clear that counter unless it gets the handle of it somehow.

joyeecheung avatar Dec 08 '25 14:12 joyeecheung