Conversation
|
Very interested in this! |
|
Very painful is this |
|
Do or do not, there is no try... |
5e29edf to
c30b66b
Compare
CodSpeed Performance ReportMerging #12419 will improve performances by 19.16%Comparing Summary
Benchmarks breakdown
|
|
The last commit where we use |
|
If anyone is interested in fixing some lifetime issues, feel free to PR it (or directly push to this branch). I might drop the last commit again because the result isn't promising enough. |
|
Overall this is very painful for marginal wins. I wonder if an alternative AST structure that uses indices would be a better approach, similar to what Zig does https://ziglang.org/download/0.8.0/release-notes.html#Reworked-Memory-Layout Let's say we have #[derive(Debug, Clone)]
struct SyntaxNode {
kind: NodeKind,
parent: NodeIndex,
children: Range<NodeIndex>,
extra_data: Range<ExtraDataIndex>
}and struct SyntaxTree {
nodes: IndexVec<NodeIndex, SyntaxNode>,
extra_data: IndexVec<ExtraDataIndex, ExtraData??>
}
struct Parsed {
root: SyntaxNode,
tree: Arc<SyntaxTree>
}We could then have AST facade nodes similar to Biome/RustAnalzyer #[derive(Clone)]
struct IfStatement {
syntax: SyntaxNode,
tree: Arc<SyntaxTree>
fn test(&self) -> Expression {
Expression::cast_unwrap(self.children(&tree)[0])
}
fn body(&self) -> Suite {
Suite::cast_unwrap(self.children(&tree)[1])
}
fn children(&self) -> &[SyntaxNode] {
self.syntax.children(&self.tree)
}It would require some changes to our AST, most notably that a node can no longer contain a sequence of children. Instead, it would need a single list node (e.g. suite) that wraps the list of children. |
|
I still think this is interesting and if anyone wants to work on this, feel free to pick it up. But I don't have the required time to finish this anytime soon. |
This doesn't make full use of the bump allocator yet. We should also
bumpalo:Vecinstead ofVecexcept in places where the vec can become large (mainly suite?)alloc_strin the parser (which copies over the strings)Intto store a&'ast strSuite) store any heap allocated data. ReplaceBox<'ast, T>with&'ast mut T.FIXME:
I think the current implementation leaks memory. We need to wrap all
StmtinBoxPerformance results
Linux
Baseline is this PR
Windows