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

perf(query eval): change ColumnOp to use row positions instead of field names + allocate less in the common case #1113

Open
joshua-spacetime opened this issue Apr 18, 2024 · 2 comments
Assignees

Comments

@joshua-spacetime
Copy link
Collaborator

Expression evaluation accounts for over a third of incremental query evaluation for certain performance benchmarks

Image

We should introduce a more optimized version of ColumnOp. This more optimal representation should have (1) row positions instead of field names, and (2) a flatter structure.

enum Expr {
    Pos(u32),
    Val(AlgebraicValue),
    BinStatic(u32, OpCmp, AlgebraicValue)
    BinDyn(Box<Expr>, OpCmp, Box<Expr>),
    Log(OpLogic, Vec<Expr>),
}

Every variant of Query that contains references to ColumnOp, we should replace with this new representation. Specifically the IndexJoin and Select variants.

@Centril
Copy link
Contributor

Centril commented Apr 18, 2024

We can also add a variant LogCmp(OpLogic, OpCmp, ColList, Vec<AlgebraicValue>), which is even more optimal for the common case of a = x && b = y, ... case.

@bfops bfops removed the performance label Apr 22, 2024
@cloutiertyler cloutiertyler changed the title Optimize expression evaluation Optimize expression evaluation by changing ColumnOp enum to use row positions instead of field names May 3, 2024
@Centril Centril changed the title Optimize expression evaluation by changing ColumnOp enum to use row positions instead of field names perf(query eval): change ColumnOp to use row positions instead of field names + allocate less in the common case May 3, 2024
@Centril
Copy link
Contributor

Centril commented May 13, 2024

The "use row positions" part is done by #1207.

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

3 participants