Skip to content

feat(core): new task graph without rules and merging #55

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

Closed
wants to merge 73 commits into from

Conversation

yliang412
Copy link
Member

@yliang412 yliang412 commented Apr 5, 2025

Problem

We need to adapt the optimizer implementation to use the new task graph design.

Summary of changes

  • Added task creation and ensuring logic.
  • Added an in-memory memo implementation, with certain operations mocked.
    • The memo merge is naive and does not trigger recursive merges.
  • Added new client interface and showcase inter-query optimization.
  • Added tests that optimize based on a preloaded memo + 0 rules.

Future work

Merging is still WIP.

@codecov-commenter
Copy link

codecov-commenter commented Apr 5, 2025

Codecov Report

Attention: Patch coverage is 38.64900% with 1435 lines in your changes missing coverage. Please review.

Project coverage is 79.2%. Comparing base (07f8873) to head (15ec949).

Files with missing lines Patch % Lines
optd-core/src/optimizer/merge.rs 0.0% 342 Missing ⚠️
optd-core/src/memo/memory.rs 53.1% 259 Missing ⚠️
optd-core/src/optimizer/tasks/mod.rs 7.6% 144 Missing ⚠️
optd-core/src/optimizer/jobs.rs 2.8% 103 Missing ⚠️
optd-core/src/optimizer/tasks/implement_expr.rs 0.0% 86 Missing ⚠️
optd-core/src/optimizer/handlers.rs 26.9% 84 Missing ⚠️
optd-core/src/optimizer/tasks/transform_expr.rs 0.0% 77 Missing ⚠️
optd-core/src/optimizer/tasks/fork_costed.rs 0.0% 45 Missing ⚠️
...d-core/src/optimizer/tasks/continue_with_costed.rs 0.0% 42 Missing ⚠️
optd-core/src/optimizer/tasks/fork_logical.rs 0.0% 42 Missing ⚠️
... and 11 more
Additional details and impacted files
Files with missing lines Coverage Δ
optd-core/src/cir/operators.rs 100.0% <100.0%> (ø)
optd-core/src/cir/rules.rs 100.0% <ø> (+100.0%) ⬆️
optd-core/src/memo/merge_repr.rs 98.6% <100.0%> (ø)
optd-core/src/optimizer/tasks/optimize_plan.rs 100.0% <100.0%> (ø)
optd-dsl/src/analyzer/map.rs 96.5% <ø> (ø)
optd-core/src/optimizer/egest.rs 33.5% <0.0%> (+33.5%) ⬆️
optd-core/src/memo/mod.rs 53.8% <53.8%> (ø)
optd-core/src/optimizer/client.rs 88.8% <88.8%> (ø)
optd-core/src/error.rs 0.0% <0.0%> (ø)
optd-core/src/optimizer/tasks/explore_group.rs 83.3% <83.3%> (ø)
... and 16 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yliang412 yliang412 changed the title [WIP] feat(core): new task graph without merging [WIP] feat(core): new task graph without rules and merging Apr 5, 2025
yliang412 and others added 28 commits April 5, 2025 20:15
Signed-off-by: Yuchen Liang <[email protected]>
Signed-off-by: Yuchen Liang <[email protected]>
Tasks are objectives. They might launch jobs that do actual work

Signed-off-by: Yuchen Liang <[email protected]>
Co-authored-by: Yuchen Liang <[email protected]>
Signed-off-by: Yuchen Liang <[email protected]>
Signed-off-by: Yuchen Liang <[email protected]>
Signed-off-by: Yuchen Liang <[email protected]>
Signed-off-by: Yuchen Liang <[email protected]>
Signed-off-by: Yuchen Liang <[email protected]>
Signed-off-by: Yuchen Liang <[email protected]>
Signed-off-by: Yuchen Liang <[email protected]>
Signed-off-by: Yuchen Liang <[email protected]>
@connortsui20 connortsui20 mentioned this pull request Apr 24, 2025
connortsui20 added a commit that referenced this pull request Apr 24, 2025
## Problem

To facilitate integration of the long-standing PR #55, this PR deletes
the entire optimizer module so that we can slowly add stuff back in.
SarveshOO7 and others added 11 commits April 26, 2025 22:22
## Problem

Some of the names in `mod.rs` are super confusing.

## Summary of changes

Changes the names of several of the types. Look at `memo/mod.rs` for the
changes, everything else is just autorenamed.
This will now allow us to merge the code much more easily into main. Furthermore, it will allow us to test the code using the DSL and it's compiled HIR.
fixed all clippy warnings except for one
@SarveshOO7 SarveshOO7 force-pushed the yuchen/optimizer-with-new-task-graph branch from 6a8690a to 1bbbd88 Compare May 1, 2025 20:15
@connortsui20 connortsui20 force-pushed the yuchen/optimizer-with-new-task-graph branch from 5fb448b to 839d8cd Compare May 2, 2025 15:49
@connortsui20
Copy link
Member

connortsui20 commented May 2, 2025

Closing while @SarveshOO7 and @yliang412 work on testing. Once that is done, we can make a new PR that will be easier to review.

DONT DELETE THE BRANCH

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.

4 participants