Backport of V8 CpuProfiler fixes/improvements

[Flarna]

@psmarshall During NodeJs diagnostics summit improvements/fixes in v8 CPU Profiler have been presented.

Is it planned/possible to backport them to Node.JS 10 or even 8 or is the difference in v8 to bit to do so?

Not sure if this is feasible, but if I could get some pointers on where to start and which commits to cherry-pick I'm happy to help in this.

[psmarshall]

Thanks for following up, here's what I'd suggest (in chronological order):

[cpu-profiler] Add source positions for inlined function calls
https://chromium.googlesource.com/v8/v8/+/af0428aca994fccfaddb2c7d723db13a2453f46b

[cpu-profiler] Reduce the size of inlining information
https://chromium.googlesource.com/v8/v8/+/a0572f0bc704a0519347c32c3dff05fc0187caa9

[cpu-profiler] Reduce size of circular queue to 512 KiB
https://chromium.googlesource.com/v8/v8/+/9fc55a9dc924d0e242e5d1da245e2c3b8c10bfa6

[cpu-profiler] De-duplicate CodeEntry objects for inline stacks
https://chromium.googlesource.com/v8/v8/+/9e1586052007ff31922dd29bbfc3c38629c3cba1

[cpu-profiler] Use compare_exchange_strong in DoSample
https://chromium.googlesource.com/v8/v8/+/4b90359c284b47ed550d959f8595d45a247da9df

[cpu-profiler] Fix flaky crashes on Windows caused by stack reads
https://chromium.googlesource.com/v8/v8/+/6e94676d15b0bc1eb592419746d15b986e344924

The first four are for reducing memory usage, the last 2 are bug fixes (probably not a huge deal for Node because they only show up on ARM and windows respectively).

I think all of them should apply to 8 & 10 cleanly - there wasn't much change between 8 & 10 in the profiler, and everything that I've backported to 10 I've also backported to 8.

[Flarna]

Thanks! I will take a look on my next free slot.

[psmarshall]

Great, thanks for working on this. Here is the fix for the shutdown delay, might want that as well:

[cpu-profiler] Wait on a condition variable in the sampling thread to enable quicker shutdowns
https://chromium.googlesource.com/v8/v8/+/6188533d64909b32580173f8543e926243f7fe9b

[Flarna]

I tried to backport them but even the first patch doesn't apply clean on v11 nor v10. I tried to fix the conflicts but it seems it's not working correct for Node 10 as memory consumption increases.

I used the crazy sample from #23070 (comment) and for NodeJs 10 the VM Heap after the usecase is ~40MB instead 11MB without the patches (tested on windows).

NodeJS 11 shows the higher mem consumption in general in this case, independent of the patches. The peak memory consumption went down so this looks ok.

See my fork of node regarding the current state:
NodeJs 10: https://github.com/Flarna/node/tree/v10-backport-cpu-profiler
NodeJs 11: https://github.com/Flarna/node/tree/v11-backport-cpu-profiler

Not sure if it makes sense that I continue here as my experience of V8 inner workings is really limited.

[psmarshall]

I will have a go at the backporting, maybe there are some other intermediate patches we need too.

[psmarshall]

I backported all those changes to 10 and resolved the conflicts, could you use my branch to check the memory situation? https://github.com/psmarshall/node/tree/v10-profiler-memory. I made the patch against v10.x-staging.

I'll also try to measure using the sample server I used before.

[psmarshall]

Hmm I can't reproduce the memory savings with that patch. I can't think of what is different to the first time I took the measurements though. I'll have to look into it more.

[jasnell]

Does this need to remain open at this point?

[Flarna]

I don't think that will happen anymore as NodeJs 8 is EOL and 10 is in maintenance.