Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for node 22 #169

Merged
merged 5 commits into from
May 8, 2024
Merged

Add support for node 22 #169

merged 5 commits into from
May 8, 2024

Conversation

nsavoire
Copy link

@nsavoire nsavoire commented Apr 26, 2024

What does this PR do?
Add asan/valgrind tests on node 22.
Add workaround for wall profiler test on node 22 / MacOS.

Copy link

github-actions bot commented Apr 26, 2024

Overall package size

Self size: 7.42 MB
Deduped: 7.79 MB
No deduping: 7.79 MB

Dependency sizes

name version self size total size
source-map 0.7.4 226 kB 226 kB
pprof-format 2.1.0 111.69 kB 111.69 kB
p-limit 3.1.0 7.75 kB 13.78 kB
delay 5.0.0 11.17 kB 11.17 kB
node-gyp-build 3.9.0 8.81 kB 8.81 kB

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@pr-commenter
Copy link

pr-commenter bot commented Apr 26, 2024

Benchmarks

Benchmark execution time: 2024-05-07 20:57:53

Comparing candidate commit 74dde8d in PR branch nsavoire/node22 with baseline commit 5fc51cb in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 60 metrics, 30 unstable metrics.

@nsavoire nsavoire added the semver-minor Usually minor non-breaking improvements label Apr 26, 2024
Qard
Qard previously approved these changes Apr 30, 2024
@nsavoire nsavoire closed this May 7, 2024
@nsavoire nsavoire reopened this May 7, 2024
@nsavoire nsavoire marked this pull request as draft May 7, 2024 08:38
szegedi and others added 3 commits May 7, 2024 22:33
With node 22, many deopt events are generated by `setContext` call in
`fn0`.
On MacOS, `v8::TimeTicks::Now` has a resolution of ~42us because
`mach_absolute_time` ticks (a tick is ~42ns) conversion to microseconds
is done in such a way that drops the 3 least significant digits.
This two facts lead to samples having identical timestamps, and
incorrectly matched contexts.
Workaround here just ensures that after deopt event caused by `setContext`,
no sample in `fn1` is immediately taken.
@nsavoire nsavoire marked this pull request as ready for review May 7, 2024 20:54
@nsavoire nsavoire requested a review from Qard May 7, 2024 20:57
@nsavoire nsavoire changed the title Test on node 22 Add support for node 22 May 7, 2024
@nsavoire nsavoire merged commit 918c1bd into main May 8, 2024
51 checks passed
@nsavoire nsavoire deleted the nsavoire/node22 branch May 8, 2024 07:41
nsavoire added a commit that referenced this pull request May 8, 2024
* Test on node 22

* Test improvements

* Update nan to 2.19.0 for Node 22

* Fix wall profiler test for node 22 on MacOS

With node 22, many deopt events are generated by `setContext` call in
`fn0`.
On MacOS, `v8::TimeTicks::Now` has a resolution of ~42us because
`mach_absolute_time` ticks (a tick is ~42ns) conversion to microseconds
is done in such a way that drops the 3 least significant digits.
This two facts lead to samples having identical timestamps, and
incorrectly matched contexts.
Workaround here just ensures that after deopt event caused by `setContext`,
no sample in `fn1` is immediately taken.

* Fix eslint warning

---------

Co-authored-by: Attila Szegedi <attila.szegedi@datadoghq.com>
nsavoire added a commit that referenced this pull request May 16, 2024
* Test on node 22

* Test improvements

* Update nan to 2.19.0 for Node 22

* Fix wall profiler test for node 22 on MacOS

With node 22, many deopt events are generated by `setContext` call in
`fn0`.
On MacOS, `v8::TimeTicks::Now` has a resolution of ~42us because
`mach_absolute_time` ticks (a tick is ~42ns) conversion to microseconds
is done in such a way that drops the 3 least significant digits.
This two facts lead to samples having identical timestamps, and
incorrectly matched contexts.
Workaround here just ensures that after deopt event caused by `setContext`,
no sample in `fn1` is immediately taken.

* Fix eslint warning

---------

Co-authored-by: Attila Szegedi <attila.szegedi@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor Usually minor non-breaking improvements
Projects
None yet
3 participants