Skip to content

Conversation

@christian-schilling
Copy link
Member

@christian-schilling christian-schilling commented Feb 3, 2022

Changing just the prefix part of a mapping in very large
workspaces resulted in very expensive filtering that can be
avoided by applying these additional rules.

@christian-schilling christian-schilling changed the title Subtract opt Add more optimiser rules for subtract Feb 3, 2022
Copy link
Collaborator

@LMG LMG left a comment

Choose a reason for hiding this comment

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

You need to update the test and I have a few questions but otherwise looks good to me

}
}

fn prefix_of(op: Op) -> Filter {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure about the name here... This gets the last of a chain, if it is a prefix?

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe last_prefix?

Op::Chain(a, to_filter(Op::Subtract(b, d)))
}
(_, b) if prefix_of(b.clone()) != to_filter(Op::Nop) => {
Op::Subtract(af, last_chain(to_filter(Op::Nop), to_filter(b.clone())).0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

here you are redoing the same operation as in prefix_of. Might be optimizable?

Changing just the prefix part of a mapping in very large
workspaces resulted in very expensive filtering that can be
avoided by applying these additional rules.
@LMG LMG enabled auto-merge (squash) February 4, 2022 09:01
@LMG LMG merged commit 0b87160 into master Feb 4, 2022
@LMG LMG deleted the subtract-opt branch February 4, 2022 09:04
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