feat: speed up transformation of debug messages#3814
Closed
TomAFrench wants to merge 5 commits intomasterfrom
Closed
feat: speed up transformation of debug messages#3814TomAFrench wants to merge 5 commits intomasterfrom
TomAFrench wants to merge 5 commits intomasterfrom
Conversation
TomAFrench
commented
Dec 15, 2023
5 tasks
Member
Author
|
Superceded by #3815 |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 15, 2023
# Description Same as #3814 but with a different approach, by building the map of old to new inside AcirTransformationMap ## Problem\* Resolves #3809 ## Summary\* ## Additional Context ## Documentation\* Check one: - [ ] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [ ] I have tested the changes locally. - [ ] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
4 tasks
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 15, 2023
# Description ## Problem\* Resolves <!-- Link to GitHub Issue --> ## Summary\* This pulls across the housekeeping changes from #3814 which weren't included in #3815 ## Additional Context ## Documentation\* Check one: - x ] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem*
Resolves #3809
Summary*
This PR modifies the algorithm for how we map the debug information when we optimize/transform the circuit such that it scales linearly with the number of opcodes rather than quadratically.
Previously we would:
This then has to search through the entire list of opcodes once per opcode. We know that optimisation/transformation doesn't reorder opcodes so a lot of this work is unnecessary, we can instead build up a reverse mapping in a single pass and then just use that.
I've changed the debug info transformation process to build a mapping from an old opcode location to the set of corresponding new opcode locations once, we can then just iterate through all of the opcode locations and map them to the new opcode locations.
There's still some issues in this code. We were (and still are) treating all opcodes with the same ACIR index as coming from the same location. This likely means we're either: a) not mapping brillig opcodes correctly or b) unnecessarily mapping brillig opcodes multiple times. I'm not too bothered about this for this PR as the behaviour is unchanged and it's more important to get the speed increase on master rather than chase potential improvements.
Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmton default settings.