Skip to content

Conversation

@maxbrunsfeld
Copy link
Contributor

@maxbrunsfeld maxbrunsfeld commented Jun 1, 2018

Problem

Previously, Node objects were implemented using Nan::ObjectWrap. Each JavaScript Node instance held a reference to a C++ TSNode instance which was allocated separately. APIs like Node.parent and Node.firstChild would instantiate nodes using V8's C++ API, which is significantly slower than instantiating objects using new in JavaScript.

Solution

Now, Nodes are plain JavaScript objects which directly store the fields of TSNode as simple JavaScript Numbers, except for the tree field, which is now stored as a reference to the JavaScript Tree wrapper object. When we want to pass a JavaScript Node to a native tree-sitter API, we transfer the node's fields to C++ via a shared ArrayBuffer and reassemble the corresponding TSNode instance in C++. For methods that return nodes, we do the same thing in reverse.

By avoiding instantiating JS objects from C++, this approach significantly speeds up tree traversals.

Node Reference Equality

Another benefit of moving the instantiation of nodes from C++ to JavaScript is that it's now very simple to memoize the nodes, so that we never get two Node instances that conceptually represent the same node. Previously, there were cases where I wanted to check for node equality, so I had added an id property to nodes that I could compare instead. Now, the id method is unnecessary and we can simply use the === operator.

class SyntaxNode {
constructor(tree) {
this.tree = tree;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class should replace Node in the .d.ts file right? Along with all it's info

@maxbrunsfeld maxbrunsfeld merged commit a86797e into master Jun 1, 2018
@maxbrunsfeld maxbrunsfeld deleted the optimize-node-api branch June 1, 2018 02:35
verhovsky added a commit to verhovsky/node-tree-sitter that referenced this pull request Mar 19, 2023
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