Skip to content

Conversation

@bianhq
Copy link
Contributor

@bianhq bianhq commented Jul 22, 2019

Review PixelsCacheReader.

@bianhq bianhq merged commit 515e0c5 into pixelsdb:master Jul 22, 2019
int bytesMatchedInNodeFound = 0;

// get root
// TODO: does root node have an edge?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes. root node can also have an edge.
The first entry in the tree is stored in the root node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. root node can also have an edge.
The first entry in the tree is stored in the root node.

Where is the root edge processed?

indexFile.getBytes(currentNodeOffset + 4 + currentNodeChildrenNum * 8,
currentNodeEdge, 0, currentNodeEdgeSize);
dramAccessCounter++;
// TODO: numEdgeBytes seems redundant.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah. It can be replaced by currentNodeEdgeSize.

dramAccessCounter++;
currentNodeChildrenNum = currentNodeHeader & 0x000001FF;
currentNodeEdgeSize = (currentNodeHeader & 0x7FFFFE00) >>> 9;
// TODO: does max length of edge = 12? can we move currentNodeEdge allocation out before this loop?
Copy link
Collaborator

Choose a reason for hiding this comment

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

The max length of edge is far larger than 12. It takes 22 bits for the size of edge in the node.

Node Header: isKey(1 bit) + edgeSize(22 bits) + childrenSize(9 bits).

I think we can move currentNodeEdge out of the loop and make it a buffer only allocated once, if we make it large enough.

Copy link
Contributor Author

@bianhq bianhq Jul 23, 2019

Choose a reason for hiding this comment

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

The max length of edge is far larger than 12. It takes 22 bits for the size of edge in the node.

Node Header: isKey(1 bit) + edgeSize(22 bits) + childrenSize(9 bits).

I think we can move currentNodeEdge out of the loop and make it a buffer only allocated once, if we make it large enough.

What is an edge? is it the key part to match on a node? 22bit -> 4MB edge size, it is so large.
Currently, we have search key = 8 byte block id + 2 byte row group id + 2 byte column id.

currentNodeEdgeSize = (currentNodeHeader & 0x7FFFFE00) >>> 9;
// TODO: does max length of edge = 12? can we move currentNodeEdge allocation out before this loop?
byte[] currentNodeEdge = new byte[currentNodeEdgeSize];
// TODO: can we get header, edge and children of a node in one memory access?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently, no, we can't do this.
Since the size of edge and children are all stored in header.
We cannot access edge and children without accessing the header first.

Copy link
Contributor Author

@bianhq bianhq Jul 23, 2019

Choose a reason for hiding this comment

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

Currently, no, we can't do this.
Since the size of edge and children are all stored in header.
We cannot access edge and children without accessing the header first.

I see. but can we merge the children and edge into one memory access?

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.

2 participants