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

Support for node v12.x #8

Closed
dhessler opened this issue Sep 1, 2019 · 8 comments
Closed

Support for node v12.x #8

dhessler opened this issue Sep 1, 2019 · 8 comments

Comments

@dhessler
Copy link

dhessler commented Sep 1, 2019

Hello,

I am running node v12.x, which includes improved async stack traces.
I'm trying to debug a potential memory leak in one of my applications, where taking snapshots is consistently crashing the process, and found this library which looks really promising.

Unfortunately the build is failing on node 12.7.0; I get a little further after naively updating all dependencies to their latest versions:

npm install

> [email protected] install ******/node-oom-heapdump/node_modules/gc-stats
> node-pre-gyp install --fallback-to-build

node-pre-gyp WARN Using needle for node-pre-gyp https download
node-pre-gyp WARN Tried to download(403): https://node-binaries.s3.amazonaws.com/gcstats/v1.4.0/Release/node-v72-darwin-x64.tar.gz
node-pre-gyp WARN Pre-built binaries not found for [email protected] and [email protected] (node-v72 ABI, unknown) (falling back to source compile with node-gyp)
  CXX(target) Release/obj.target/gcstats/src/gcstats.o
  SOLINK_MODULE(target) Release/gcstats.node
  COPY ******/node-oom-heapdump/node_modules/gc-stats/build/gcstats/v1.4.0/Release/node-v72-darwin-x64/gcstats.node
  TOUCH Release/obj.target/action_after_build.stamp

> [email protected] install ******/node-oom-heapdump
> node-gyp rebuild

  CXX(target) Release/obj.target/node_oom_heapdump_native/lib/node_oom_heapdump_native.o
../lib/node_oom_heapdump_native.cc:77:44: error: no matching member function for call to
      'ToString'
  String::Utf8Value fArg(isolate, args[0]->ToString());
                                  ~~~~~~~~~^~~~~~~~
/Users/david/.node-gyp/12.7.0/include/node/v8.h:2535:44: note: candidate function not
      viable: requires single argument 'context', but no arguments were provided
  V8_WARN_UNUSED_RESULT MaybeLocal<String> ToString(
                                           ^
/Users/david/.node-gyp/12.7.0/include/node/v8.h:2551:31: note: candidate function not
      viable: requires single argument 'isolate', but no arguments were provided
                Local<String> ToString(Isolate* isolate) const);
                              ^
../lib/node_oom_heapdump_native.cc:83:27: error: no matching member function for call to
      'BooleanValue'
  addTimestamp = args[1]->BooleanValue();
                 ~~~~~~~~~^~~~~~~~~~~~
/Users/david/.node-gyp/12.7.0/include/node/v8.h:2566:8: note: candidate function not
      viable: requires single argument 'isolate', but no arguments were provided
  bool BooleanValue(Isolate* isolate) const;
       ^
/Users/david/.node-gyp/12.7.0/include/node/v8.h:2569:51: note: candidate function not
      viable: requires single argument 'context', but no arguments were provided
                V8_WARN_UNUSED_RESULT Maybe<bool> BooleanValue(
                                                  ^
2 errors generated.
make: *** [Release/obj.target/node_oom_heapdump_native/lib/node_oom_heapdump_native.o] Error 1
gyp ERR! build error
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/usr/local/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:266:23)
gyp ERR! stack     at ChildProcess.emit (events.js:203:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:272:12)
gyp ERR! System Darwin 18.7.0
gyp ERR! command "/usr/local/bin/node" "/usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd ******/node-oom-heapdump
gyp ERR! node -v v12.7.0
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] install: `node-gyp rebuild`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

Do you expect to add support for node 12 anytime soon?
Thanks!

@paulrutter
Copy link
Contributor

Hi @dhessler,

Yes, the company i work for is planning to move to Node 12 when it becomes LTS in october.
Part of that upgrade is making sure this module works on 12 as well.

So yes, i expect to add support for Node 12 in the near future.

Regards,

@paulrutter
Copy link
Contributor

paulrutter commented Sep 2, 2019

I haven't been able to test it thoroughly, but i managed to modify the native code to compile for Node.js 12. I've committed my code in a branch (node12) and published a "beta" version on npm, to test with.

See https://www.npmjs.com/package/node-oom-heapdump/v/1.2.0-beta.0.
Let me know what your experiences are.

UPDATE:

<--- Last few GCs --->

[83:0x3af7320]     1265 ms: Mark-sweep 79.9 (81.3) -> 79.6 (80.8) MB, 59.9 / 0.0 ms  (+ 22.0 ms in 16 steps since start of marking, biggest step 17.4 ms, walltime since start of marking 98 ms) (average mu = 0.460, current mu = 0.169) allocation failure sc[83:0x3af7320]     1358 ms: Mark-sweep 79.8 (80.8) -> 79.4 (80.8) MB, 75.9 / 0.0 ms  (+ 11.8 ms in 2 steps since start of marking, biggest step 11.8 ms, walltime since start of marking 92 ms) (average mu = 0.300, current mu = 0.053) allocation failure sca

<--- JS stacktrace --->
Cannot get stack trace in GC.


#
# Fatal error in , line 0
# unreachable code
#
#
#
#FailureMessage Object: 0x7fe869953880[2019-09-02T13:15:45+01:00]

It looks like i'll need to dig a little deeper; creating a heapdump through the v8 API causes a FATAL error on Node12, where this used to work earlier.

@dhessler
Copy link
Author

dhessler commented Sep 3, 2019

wow thanks @paulrutter for the quick investigation! definitely happy to beta test when ready.

@paulrutter
Copy link
Contributor

I'm having a hard time getting it to work on Node.js 12.x.
See nodejs/node#27552 (comment).

I think i'll need help from the v8 team in order to proceed.
It seems that the API's haven't changed, but the context in which the API is called no longer seems to work on Node.js 12.

To be continued.

@paulrutter
Copy link
Contributor

@dhessler after investigation, it looks like the combination of node startup flags i was using for my test case, caused the module to be unstable on nodejs 12.
Although this is not as it should be (i'm waiting on response from the nodejs team), it does work when not using those flags.

So, i would suggest we start using it on Node 12, to gather hands on experience with it.
If you encounter any issues, please let me know.

I will start using it in the next couple of months for sure.
Version 1.2.0 is now published to npm :)

@paulrutter
Copy link
Contributor

I'm closing the issue for now.
Let me know when you encounter any issues by filing a new issue.

@jorisw
Copy link

jorisw commented Jul 23, 2020

@paulrutter Which were the flags causing Node 12 to run badly? I'm having the same problem with v2.0.0 on Node 12.10.

@paulrutter
Copy link
Contributor

See fac8e92
These were:

  • --optimize_for_size
  • --always_compact

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants