-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
os: name every function OS module #9250
Conversation
@lprasanthi Can you please change the commit message, as per these guidelines? |
There are too many anonymous functions in the source code which makes the debugging frustrating. We are fixing this issue by naming every anonymous function in all the modules.As part of which,this commit contains changes to os module
7969f31
to
9e63b3b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, but I'm afraid I'm opposed to this as it changes the code for no benefit.
Relevant stack trace line before:
at Object.exports.arch (os.js:27:9)
..and with this change:
at Object.arch (os.js:27:9)
I'm not seeing the benefit here. Sorry.
If you want to find anonymous functions where naming them will improve the stack trace, look for code like this:
some.functionCall(arg1, arg2, function() {
//this function needs a name
});
Actually, I suspect this can be salvaged by creating an object in the module named |
@Trott this is about heap snapshots and such, not stacktraces. |
If I'm not mistaken, the same issue applies to heap snapshots.
With the change here, it will appear as simply My understanding of how to read heap snapshots in Chrome Dev Tools is limited, so I may be missing something. Corrections are welcome. But if I'm not completely off base here, then I would think at least that my suggestion in #9250 (comment) is more the way to go. |
@Trott any idea on how to remove the mod=exports;
mod.fn = () => console.trace();
fn(); but curiously, not when immediatly invoked: mod=exports;
(mod.fn = () => console.trace())(); |
@silverwind Not sure, actually. Haven't had time to look too closely, but if someone else knows or figures it out, please post here. :-D |
Any updates on this? |
There hasn't been any activity here. I'm closing this. Feel free to reopen (or ping a collaborator) if I closed this in error. |
Checklist
make -j8 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
os
Description of change
There are too many anonymous functions in the source code which makes heap debugging frustrating.
Naming functions in os module as part of this issue