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 JS_DumpObjects support #353

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

HarlonWang
Copy link

@HarlonWang HarlonWang commented Sep 19, 2024

To analyze memory leaks and usage more accurately, it would be great to expose JS_DumpObjects .
image

@saghul
Copy link
Contributor

saghul commented Sep 19, 2024

It would be nice if the function got a FILE *f parameter so it would work with any stream other than stdout too.

@HarlonWang
Copy link
Author

I also think that's better, but the changes to the original logic will be quite extensive. The following methods will all need adjustments(add a FILE *f parameter).

static __maybe_unused void JS_DumpAtoms(JSRuntime *rt);
static __maybe_unused void JS_DumpString(JSRuntime *rt, const JSString *p);
static __maybe_unused void JS_DumpObjectHeader(JSRuntime *rt);
static __maybe_unused void JS_DumpObject(JSRuntime *rt, JSObject *p);
static __maybe_unused void JS_DumpGCObject(JSRuntime *rt, JSGCObjectHeader *p);
static __maybe_unused void JS_DumpValueShort(JSRuntime *rt, JSValueConst val);
static __maybe_unused void JS_DumpValue(JSContext *ctx, JSValueConst val);

I can try making these changes, but I'm not sure if they'll be accepted.

Do you have any better suggestions? @saghul

By the way, I often use this method to analyze leaked objects. in my use case, I achieved writing to a file by proxying stdout, so it would be great if this could be officially supported.

@saghul
Copy link
Contributor

saghul commented Sep 19, 2024

I'd accept the change in the friendly fork though ;-)

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

Successfully merging this pull request may close these issues.

3 participants