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 node's heap profiling #4461

Open
julienw opened this issue Feb 8, 2023 · 4 comments · May be fixed by #4467
Open

Support node's heap profiling #4461

julienw opened this issue Feb 8, 2023 · 4 comments · May be fixed by #4467
Assignees
Labels
assigned A developer has requested to work on this issue. help wanted Things ready to be worked on by anyone. Issues must include instructions on how to complete the task profile data Issues related to the profile format, data structure, or profile upgraders

Comments

@julienw
Copy link
Contributor

julienw commented Feb 8, 2023

Node can output a heap profile file when run with --heap-prof. It would be good to be able to import it into the profiler UI.
Here is an example:
Heap.20230208.132746.551039.0.002.heapprofile.gz

┆Issue is synchronized with this Jira Task

@julienw julienw added help wanted Things ready to be worked on by anyone. Issues must include instructions on how to complete the task profile data Issues related to the profile format, data structure, or profile upgraders labels Feb 8, 2023
@krsh732
Copy link
Contributor

krsh732 commented Feb 9, 2023

Can I try this? I did some digging around and found: https://source.chromium.org/chromium/chromium/src/+/main:v8/include/js_protocol.pdl;l=696-729

So maybe using the existing importers as reference I might be able to get something working.

@julienw
Copy link
Contributor Author

julienw commented Feb 9, 2023

Oh yes, please !

@julienw julienw added the assigned A developer has requested to work on this issue. label Feb 9, 2023
@julienw
Copy link
Contributor Author

julienw commented Feb 9, 2023

I'll try to give some more context to the memory tooling in the profiler UI. You can read an introduction in our docs already: https://profiler.firefox.com/docs/#/./memory-allocations

The simple memory usage (just incrementing or decrementing) is reported in counters, that gives the memory track. But I believe that the data in these heap profiles are more detailed than this, and we want to use the more advanced mechanisms.

Then the 2 other ways are the Native Allocations and the JS Allocations. In Gecko they're reported in markers, but they're converted to other tables in the processed format. I believe that's what we should target.

You can read some more about these types in:

/**
* This variant is the original version of the table, before the memory address
* and threadId were added.
*/
export type UnbalancedNativeAllocationsTable = {|
time: Milliseconds[],
// "weight" is used here rather than "bytes", so that this type will match the
// SamplesLikeTableShape.
weight: Bytes[],
weightType: 'bytes',
stack: Array<IndexIntoStackTable | null>,
length: number,
|};
/**
* The memory address and thread ID were added later.
*/
export type BalancedNativeAllocationsTable = {|
...UnbalancedNativeAllocationsTable,
memoryAddress: number[],
threadId: number[],
|};
/**
* Native allocations are recorded as a marker payload, but in profile processing they
* are moved to the Thread. This allows them to be part of the stack processing pipeline.
* Currently they include native allocations and deallocations. However, both
* of them are sampled independently, so they will be unbalanced if summed togther.
*/
export type NativeAllocationsTable =
| UnbalancedNativeAllocationsTable
| BalancedNativeAllocationsTable;

They're inserted in the thread as optional properties:

samples: SamplesTable,
jsAllocations?: JsAllocationsTable,
nativeAllocations?: NativeAllocationsTable,

Our support for memory profiling is very "adhoc" in that it's very much tailored to the current data we get from the gecko profiler. For example we only support deallocations / retained memory support with the nativeAllocations data.

But getting an importer to work with any of the chrome data would already be a fantastic achievement. Once we have the importer we can tweak things a lot more easily, and then make the related code more generic.

Hope this helps :-)

@krsh732 krsh732 linked a pull request Feb 9, 2023 that will close this issue
2 tasks
@krsh732
Copy link
Contributor

krsh732 commented Feb 10, 2023

@julienw I see some words and lines that are familiar to what I used yesterday, so hopefully that's a sign that I haven't completely messed up 🙂 I've put up draft PR #4467 for some feedback when you have time. Let me know if things are looking even remotely close to OK, in which case I can add a snapshot test on top of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned A developer has requested to work on this issue. help wanted Things ready to be worked on by anyone. Issues must include instructions on how to complete the task profile data Issues related to the profile format, data structure, or profile upgraders
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants