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

Remove AC matching #67

Merged
merged 3 commits into from
Jan 19, 2019
Merged

Remove AC matching #67

merged 3 commits into from
Jan 19, 2019

Conversation

HarrisonGrodin
Copy link
Owner

It seems like numerous performance issues can be attributed to the costliness of AC pattern matching.

The match function is called for every application of a pattern-based rule on every subterm of every intermediate term. Thus, matching clearly must be efficient during normalization - for example, a recent coverage build shows that over the course of the test suite, we:

  • call the top-level match method 4 million times
  • call associative matching 1 million times
  • call commutative matching 280k times
  • construct nearly 7 million Match objects

In addition to the algorithmic complexity associated with associative and commutative matching, allowing for the storage of multiple matches using nested data structures increases the number of allocations performed. Additionally, the use of this specialized algorithm in such a fundamental procedure severely limits our ability to apply other modern research techniques to Rewrite.


This PR removes AC pattern matching, substituting it for more performant normalization strategies when AC is necessary.

  • Remove AC pattern matching
  • Add custom rules reimplementing the functionalities where necessary

@HarrisonGrodin HarrisonGrodin added feature New feature or request help wanted Extra attention is needed improvement Improvement to existing feature performance Related to code speed/performance labels Jan 3, 2019
@HarrisonGrodin HarrisonGrodin added this to the 0.1 milestone Jan 3, 2019
@HarrisonGrodin HarrisonGrodin self-assigned this Jan 3, 2019
@HarrisonGrodin
Copy link
Owner Author

cc: @MasonProtter

@HarrisonGrodin HarrisonGrodin mentioned this pull request Jan 6, 2019
@HarrisonGrodin
Copy link
Owner Author

How do we want to handle normalization of AC operators? Explicit lazy AC matching?

@HarrisonGrodin HarrisonGrodin changed the base branch from master to move/simplify January 19, 2019 02:50
@HarrisonGrodin HarrisonGrodin changed the title [WIP/RFC] Remove AC matching Remove AC matching Jan 19, 2019
@HarrisonGrodin HarrisonGrodin merged commit 677b3db into move/simplify Jan 19, 2019
@HarrisonGrodin HarrisonGrodin deleted the refactor/ac branch January 19, 2019 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request help wanted Extra attention is needed improvement Improvement to existing feature performance Related to code speed/performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants