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

Upstream llnode to V8 #64

Closed
natorion opened this issue Jan 30, 2017 · 22 comments
Closed

Upstream llnode to V8 #64

natorion opened this issue Jan 30, 2017 · 22 comments

Comments

@natorion
Copy link

I recently had a chat with @indutny about llnode and concluded that upstreaming it to V8 might make sense. Let's start the discussion if it really makes sense and how it roughly could be accomplished.

cc @ofrobots @bmeurer @hashseed

@mhdawson
Copy link
Member

Please include me in the discussion.

@indutny
Copy link
Member

indutny commented Jan 30, 2017

The pros that I see:

  • Tighter integration with V8's changes

Cons:

  • Google-specific toolchain (i.e. GN, or something) that will be used

@Fishrock123
Copy link
Member

Won't this make it difficult to contribute to?

@indutny
Copy link
Member

indutny commented Jan 30, 2017

@Fishrock123 this may be another downside.

@natorion perhaps, the best way forward on this for now would be to try to integrate this with V8's test suite. I suppose this means porting the tests to d8 (or writing a wrapper), but whatever the required changes are - they can be upstreamed back to this repo at any time. Also, I can help you with anything you need.

@mhdawson
Copy link
Member

I think one of the main dependencies that llnode depends on is the metadata which is tied to v8, would integrating that or whatever generation/upkeep is needed for that metadata into v8 be a good first step ? Then the existing project could be used to work with cores generated from any project using v8, not just node.

@indutny
Copy link
Member

indutny commented Jan 30, 2017

@mhdawson the metadata is already integrated in V8. llnode pulls it from it, and actually works with any project using V8 at the moment (as long as post-mortem data was enabled during the build).

@misterdjules
Copy link

misterdjules commented Jan 31, 2017

What would "upstreaming llnode to V8" mean in more details? Besides llnode, there is an ecosystem of post-mortem debugging tools for V8 (IBM's IDDE, mdb_v8, etc.) and I would think that we'd want that ecosystem to thrive. How would that endeavor contribute to that (EDIT: to clarify, this is not a rethorical question, more a candid one)?

@mhdawson
Copy link
Member

@indutny ok thanks, for some reason I thought there were extra structures that we maintain for use by mdb/llnode but maybe those are just additions for node.js ?

@natorion
Copy link
Author

@indutny to clarify: You mean porting https://github.com/nodejs/llnode/tree/master/test to be runnable in d8 and importing those tests into the V8 repo? I am not sure how this might help?

@mhdawson including relevant folks was the intention of a public issue :-).

@mhdawson
Copy link
Member

In response to the comment from @misterdjules about how to support the broader set of tools, I can see how the code that pulls together the information needed by the commands would fit nicely in v8 so that when changes were made there, the corresponding changes would be made at the same time. The code that integrates into the debugger I'm less sure about. What we'd been discussing earlier was to try to get to the point were we had the implementation for commands that could be used for multiple debuggers like llnode, mdb, dbx etc. If there was a set of methods that could be registered by a debugger (for example to read memory etc.) that v8 used to pull together the information required for a particular command and a way for the debuggers to call a function for a particular command, it might allow the same command implementations from v8 to be used across the debuggers. When necessary the debugger addon project (ex llnode) could add addition commands itself as well. This might be a way to allow the node community to easily contribute new commands when needed and then have them move into v8 when they are stable and will likely need less activity.

@misterdjules
Copy link

@mhdawson

I can see how the code that pulls together the information needed by the commands would fit nicely in v8 so that when changes were made there, the corresponding changes would be made at the same time.

Can you be more specific? There's already code in V8 that extracts the metadata needed by sone of the post-mortem debuggers, see https://cs.chromium.org/chromium/src/v8/tools/gen-postmortem-metadata.py. Were you thinking of something different?

The code that integrates into the debugger I'm less sure about.

I don't understand what you mean by that.

What we'd been discussing earlier was to try to get to the point were we had the implementation for commands that could be used for multiple debuggers like llnode, mdb, dbx etc. If there was a set of methods that could be registered by a debugger (for example to read memory etc.) that v8 used to pull together the information required for a particular command and a way for the debuggers to call a function for a particular command, it might allow the same command implementations from v8 to be used across the debuggers.

Are you referring to nodejs/post-mortem#37?

@mhdawson
Copy link
Member

mhdawson commented Jan 31, 2017

My understanding that in addition to the meta data, the implementations of commands in llnode still have additional dependencies on the v8 implementation such that changes in v8 could necessitate changes in the command implementations (ie not everything is abstracted out by the metadata), it is that code that I can see a benefit to living more closely with v8.

Other parts of the implementations of the commands in llnode will be related to reading from the core dump and I believe that code uses apis from the underlying debugger to do that. It is that part that I'm less sure moving into v8 would be a win.

Richard/Howard have spent more time working on llnode commands so they can correct me if I've misunderstood. @hhellyer @rnchamberlain

part of the discussion that nodejs/post-mortem#37 is intended to capture was related to being able to re-use the code for commands across the debuggers, the goal being that we can avoid having to write/maintain commands individually for each tool.

@hhellyer
Copy link
Collaborator

hhellyer commented Feb 1, 2017

Since I’ve spend a fair while looking at both llnode and lldb here’s my thoughts. I’m probably not going to be making the decision but hopefully these will be useful to whoever does. (Feel free to ignore, refute or dispute any of them!)

llnode is a bit of a misnomer - most of the llnode code relates to exploring V8 structures using calls into lldb, llv8 would almost be a better name. (Sorry @indutny !) Apart from one function I added to quickly format the process object and tell the user what version of node they were running I don’t think there’s much that’s node specific in there. On the other hand as it is V8 specific it would be useless for a Node.js implementation built on another JavaScript runtime like Chakra.

llnode is quite tightly bound to lldb and its public SB API. Abstracting this out would be possible but I’m not sure if cross debugger support is a thing people really want or whether the community would rely on lldb for cross platform support to achieve the same aim.

lldb’s platform support is mixed and this may or may not be important to V8. Mac support is great and Linux (x86_64) is pretty good. Windows and non-x86_64 Linux is less reliable. I don’t know what the most interesting platforms are to the V8 team. If they are very interested in Windows then llnode may have less value that it does to Node.js where the most important platforms seem to be Mac and Linux. (That said the lldb team are quite welcoming to improvements, releases are frequent and Windows support seems to be being improved.)

I think it’s confusing to talk about metadata coming from the build. There are symbols containing constants that are needed to understand the V8 object layouts (and a few other bits) which are probably what people think of but those are useless without the correct algorithms for walking objects, llnode effectively reproduces the algorithms that V8 uses and that’s as essential as the constants.

How llnode handles V8 versions is interesting. llnode has no build time dependancy on either Node or V8. The binary is tied to the lldb interface it’s built against (because it’s C++) but can read core dumps from Node 4, 6, 7 etc… There is code in llnode in a few places that accounts for V8 differences and chooses which constants and/or algorithms to use. As the constants are loaded as symbols from the core dump including those needed to work out the v8 version all those decisions are made once the core dump is loaded not when llnode is built.
If llnode was delivered as part of the V8 codebase they might logically prefer to stream it by V8 version, this would let them simplify the code and might even let them pull in constants directly without using the gen-postmortem-metadata.py script.
I don’t know if this would have any implications really for Node.js, users might have to be more careful about how they obtain the right version it but I doubt it would be fatal.

@mhdawson
Copy link
Member

mhdawson commented Feb 1, 2017

@hhellyer In terms of platform support, I think what is important to us (Node community) is that platforms that Node supports (which includes windows etc.) are addressed. So I'd be considering not if v8 cares about those platforms but how integration affects the ability of the node community to continue to support our stated platforms. As you say the algorithms are linked to v8 so if we can make sure they get updated along with other v8 updates that's a good thing, and we want to do it in a way that allows us to leverage that benefit across all of the platforms supported by Node.js

@indutny
Copy link
Member

indutny commented Feb 6, 2017

Alright, there is so much to answer!

What would "upstreaming llnode to V8" mean in more details? Besides llnode, there is an ecosystem of post-mortem debugging tools for V8 (IBM's IDDE, mdb_v8, etc.) and I would think that we'd want that ecosystem to strive. How would that endeavor contribute to that (EDIT: to clarify, this is not a rethorical question, more a candid one)?

The benefit of llnode here is that it potentially can work across all supported platforms (it works everywhere except Windows at the moment, and this is fixable). Having some sort of integration with V8 simplifies management of our "bindings" to V8 internals.

Right now we track all internal representation changes in V8 from one node.js release to another to make our tools up-to-date. Having it close to the source of object's structure will make maintenance costs significantly less that what they are now.

@indutny to clarify: You mean porting https://github.com/nodejs/llnode/tree/master/test to be runnable in d8 and importing those tests into the V8 repo? I am not sure how this might help?

Yep, that's what I meant. This will help us to keep the tool up-to-date. If something is broken, we will know about it at the time of the commit that broke it 😉


@natorion will moving llnode to v8 make it easier for you to contribute or just work with it? If this is the case - what things could improved on our side to make this repo as good as the Google's would be?

@misterdjules
Copy link

@indutny

The benefit of llnode here is that it potentially can work across all supported platforms (it works everywhere except Windows at the moment, and this is fixable).

Does it work on SmartOS? Even if it worked on SmartOS, does it provide at least the same features as all other post-mortem debugging tools?

Having some sort of integration with V8 simplifies management of our "bindings" to V8 internals.

What precisely do you mean by "some sort of integration"?

Also, what precisely do you mean by "bindings"? Do you mean the algorithms used by post-mortem debugging tools to determine the nature of JS objects?

Right now we track all internal representation changes in V8 from one node.js release to another to make our tools up-to-date. Having it close to the source of object's structure will make maintenance costs significantly less that what they are now.

Do you mean that an additional API and its implementation would be added to V8 such that a program would be able to query V8 to determine e.g whether a given address in memory stores the representation of a JS object?

Again, my primary concern is that we need to support the entire existing ecosystem of post-mortem debugging tools, not just llnode. It might certainly be possible to integrate into V8 part of what current post-mortem debugging tools use to examine node core files, but this needs to be done in consultation with all parties involved. Otherwise, there's a risk of integrating something that is at best unusable by some post-mortem debugging tools, and at worse breaking them.

Currently, the rationale, goals and scope of the integration suggested in this issue are not clear and easy to understand, so it's hard to say how this would impact other post-mortem debugging tools than llnode.

@indutny
Copy link
Member

indutny commented Feb 7, 2017

@misterdjules

Does it work on SmartOS? Even if it worked on SmartOS, does it provide at least the same features as all other post-mortem debugging tools?

I haven't tested it on Solaris yet. Does lldb work there? Feature-wise, I think it provides very similar functionality to mdb.

What precisely do you mean by "some sort of integration"?

V8 could potentially export byte-code-like stubs that would abstract away some implementation details from post-mortem tools. ( @natorion does it sound interesting? 😉 )

Also, what precisely do you mean by "bindings"? Do you mean the algorithms used by post-mortem debugging tools to determine the nature of JS objects?

Yep.

Do you mean that an additional API and its implementation would be added to V8 such that a program would be able to query V8 to determine e.g whether a given address in memory stores the representation of a JS object?

I mean changing internal representation of objects in V8. This happens from time to time, and we have to keep track of such changes and have a backwards compatible code.

Currently, the rationale, goals and scope of the integration suggested in this issue are not clear and easy to understand, so it's hard to say how this would impact other post-mortem debugging tools than llnode.

I absolutely agree on this.

@hhellyer
Copy link
Collaborator

hhellyer commented Feb 7, 2017

@indutny @misterdjules - Of the platforms currently available for download from nodejs.org I don't believe LLDB supports SmartOS/Solaris or AIX.

@misterdjules
Copy link

@indutny

I haven't tested it on Solaris yet. Does lldb work there?

@hhellyer seems to point out that it doesn't.

Feature-wise, I think it provides very similar functionality to mdb.

I wouldn't think so. Does lldb support piping commands? Does lldb support CTF, walkers, etc? In other words, not only lldb seems to provide a different features set than mdb, but it would take a lot of work and time for mdb users to switch to lldb, even if it was possible.

As a result, I would think that any integration of llnode into V8 would need to consider the impact on users of other debuggers.

What precisely do you mean by "some sort of integration"?

V8 could potentially export byte-code-like stubs that would abstract away some implementation details from post-mortem tools. ( @natorion does it sound interesting? 😉 )

Is that considered "upstreaming llnode to V8", or is it a different topic? It seems to me that it's a different topic.

Do you mean that an additional API and its implementation would be added to V8 such that a program would be able to query V8 to determine e.g whether a given address in memory stores the representation of a JS object?

I mean changing internal representation of objects in V8. This happens from time to time, and we have to keep track of such changes and have a backwards compatible code.

Again, how does this relate to "upstream llnode to V8"?

@hhellyer
Copy link
Collaborator

At the risk of adding yet another option, yet another option is to put the support into lldb itself.
It does support languages other than C/C++ though I don't really know the details of how it works or how suitable it would be. It looks like someone at Google added support for Go but I don't know enough about Go or extending lldb like that to say whether that would mean adding V8 support to lldb would be a good idea.

(For example, if Go is based on llvm it would have made a lot of sense to add support to lldb but I don't know if it is.)

@bnoordhuis
Copy link
Member

For native lldb support you'd need useful debug info, a stable ABI and, probably, a stable runtime (for hooking.) Go has them, V8 doesn't.

Debug info isn't insurmountable (it already supports that, after a fashion; the --gdb switch) but the other two? I doubt it.

@indutny
Copy link
Member

indutny commented Apr 19, 2017

Closing this, please re-open if relevant.

@indutny indutny closed this as completed Apr 19, 2017
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

7 participants