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.

Sign up for GitHub

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 Node.js v20+ support #15

Merged
merged 4 commits into from
Nov 7, 2024
Merged

Add Node.js v20+ support #15

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cbc079b
Add Node.js v20 support
christianvuerings Nov 7, 2024
4364d4c
Fix indendation + add editorconfig file
christianvuerings Nov 7, 2024
2b66f40
Revert vitest conversion
christianvuerings Nov 7, 2024
6712097
Remove kGCTypeMinorMarkCompact to maintain previous behavior
christianvuerings Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .editorconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# editorconfig.org
root = true

[*]
indent_style = space
indent_size = 2
end_of_line = lf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true

[*.md]
trim_trailing_whitespace = false
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node: [ 10, 12, 14, 16, 18 ]
node: [ 18, 20, 22 ]
name: Node ${{ matrix.node }}
continue-on-error: true
steps:
Expand Down
5 changes: 2 additions & 3 deletions include.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const
magic = require('bindings')('memwatch'),
events = require('events');
const magic = require('bindings')('memwatch');
const events = require('events');

module.exports = new events.EventEmitter();

Expand Down
216 changes: 138 additions & 78 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"version": "2.0.0",
"author": "Lloyd Hilaiel (http://lloyd.io)",
"engines": {
"node": ">= 10.0"
"node": ">= 18"
},
"repository": {
"type": "git",
Expand Down Expand Up @@ -34,7 +34,7 @@
],
"dependencies": {
"bindings": "^1.5.0",
"nan": "^2.14.1"
"nan": "^2.22.0"
},
"publishConfig": {
"access": "public"
Expand Down
1 change: 1 addition & 0 deletions src/memwatch.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ NAN_GC_CALLBACK(memwatch::after_gc) {
s_stats.gcProcessWeakCallbacksTime += gcTime;
return;

case kGCTypeMinorMarkCompact:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes

../src/memwatch.cc:149:12: warning: enumeration value 'kGCTypeMinorMarkCompact' not handled in switch [-Wswitch]

Copy link

@jparise jparise Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also determine whether the case below should also consider kGCTypeMinorMarkCompact (i.e. does the "compaction" accounting behavior apply to both types):

node-memwatch/src/memwatch.cc

Line 168 in 99c84fa

if (type == kGCTypeMarkSweepCompact) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like v8 / chromium renamed kGCTypeMinorMarkCompact to kGCTypeMinorMarkSweep
chromium/chromium@257c88d#diff-11920fd94088cac09a3e53d31a3a3565501b9692fa7842fd98168536e279e002R174
https://chromium-review.googlesource.com/c/chromium/src/+/4669537

https://wingolog.org/archives/2023/12/07/the-last-5-years-of-v8s-garbage-collector

Originally called minor mark-compact or MinorMC in the commit logs, it was renamed to minor mark-sweep or MinorMS to indicate that it doesn’t actually compact. (V8’s mark-compact old-space doesn’t have to compact: V8 usually chooses to just mark in place. But we call it a mark-compact space because it has that capability.)

Also noticed that we already have this warning in the current version of our package so going to pull this change out for now to not cause any regressions: https://github.com/airbnb/node-memwatch/actions/runs/11726571196/job/32665528031#step:4:166

../src/memwatch.cc:149:11: warning: enumeration value ‘kGCTypeMinorMarkCompact’ not handled in switch [-Wswitch]
  149 |     switch(type) {
      |           ^

case kGCTypeMarkSweepCompact:
case kGCTypeAll:
break;
Expand Down