nodejs
lib: add util.getCallSite() API
This pull request introduces a new API method, util.getCallSite(), which provides a way to capture and inspect the call stack within a Node.js application. The method returns an array of CallSite objects, so developers can retrieve information about each function call in the stack trace.
Previously, they could reach the same behaviour by using Error.prepareStackTrace but, this API isn't that friendly IMO.
Codecov Report
Attention: Patch coverage is 95.91837% with 2 lines in your changes missing coverage. Please review.
Project coverage is 87.60%. Comparing base (
298dea0) to head (aa11f56). Report is 250 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/node_util.cc | 94.59% | 0 Missing and 2 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #54380 +/- ##
==========================================
- Coverage 87.61% 87.60% -0.01%
==========================================
Files 650 650
Lines 182829 182878 +49
Branches 35382 35390 +8
==========================================
+ Hits 160179 160210 +31
Misses 15931 15931
- Partials 6719 6737 +18
| Files with missing lines | Coverage Δ | |
|---|---|---|
| lib/util.js | 95.91% <100.00%> (+0.12%) |
:arrow_up: |
| src/node_util.cc | 85.77% <94.59%> (+1.61%) |
:arrow_up: |
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
I'd not include an option to ignore it in this initial version - maybe a follow up pr once it's landed?
It's not supposed to be printed via console.log because it's a CallSite representation. I will modify the API so it retrieves the stack trace from C++, and this should address this issue with the new object
lgtm
Did you do a benchmark?
I did, but without having a fair comparison I don't know if it will mean much:
From my local machine
util/get-callsite.js n=1000000: 178,641.08162235853
I suspect the C++ implementation will be slower than the JS one for this particular scenario. We need to iterate over v8::StackTrace to create objects while the JS approach does not compute the values unless the function is called. e.g: .getFileName(). So the comparison will be unfair.
But, remember that we've switched to C++ for a side-effect-less approach. I can optimize this code later.
@mcollina I have just included a benchmark for the previous approach (using Error) and as I said it will be unfair as one computes fields and the Error approach does not (return CallSite{}), so to make it more "fair" I've created a third benchmark that uses the Error approach but compute the fields. (See 8bc130c)
Results:
util/get-callsite.js method="ErrorCallSite" n=1000000: 303,889.4093489688
util/get-callsite.js method="ErrorCallSiteSerialized" n=1000000: 190,435.9778337435
util/get-callsite.js method="CPP" n=1000000: 175,429.33099106076
Commit Queue failed
- Loading data for nodejs/node/pull/54380 ✔ Done loading data for nodejs/node/pull/54380 ----------------------------------- PR info ------------------------------------ Title lib: add util.getCallSite() API (#54380) Author Rafael Gonzaga <[email protected]> (@RafaelGSS) Branch RafaelGSS:add-getCallSite -> nodejs:main Labels util, semver-minor, author ready, needs-ci, commit-queue-rebase Commits 6 - lib: add util.getCallSite() API - benchmark: add util.getCallSite bench - fixup! lib: add util.getCallSite() API - fixup! fixup! lib: add util.getCallSite() API - fixup! fixup! fixup! lib: add util.getCallSite() API - fixup! fixup! fixup! fixup! lib: add util.getCallSite() API Committers 1 - RafaelGSS <[email protected]> PR-URL: https://github.com/nodejs/node/pull/54380 Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/54380 Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]> -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last approving review: ⚠ - fixup! fixup! fixup! lib: add util.getCallSite() API ⚠ - fixup! fixup! fixup! fixup! lib: add util.getCallSite() API ℹ This PR was created on Wed, 14 Aug 2024 17:42:46 GMT ✔ Approvals: 3 ✔ - Vinícius Lourenço Claro Cardoso (@H4ad): https://github.com/nodejs/node/pull/54380#pullrequestreview-2248638869 ✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/54380#pullrequestreview-2246871973 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/54380#pullrequestreview-2247058195 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-08-22T18:07:19Z: https://ci.nodejs.org/job/node-test-pull-request/61355/ - Querying data for job/node-test-pull-request/61355/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/10531270448