Avoid using toAbsoluteName so much in rewriter
#1263
Labels
performance
Issues that involve or include performance problems
tech debt
Issues that document or involve technical debt
Comments
brianhuffman
added
tech debt
brianhuffman
pushed a commit
that referenced
this issue
Apr 27, 2021
Fixes #1263. We now use `identBaseName` instead of `identText` in the `termToPat` definition. While `identText` concatenates the module name and base name, `identBaseName` just returns the base name directly. Using only the base name means that there is a chance of name collisions between constants that differ only by module name. However, term nets are only used as an approximate filter prior to term matching; it is not a problem if a few extra hits are returned from time to time. Ultimately it might be better to replace the use of `Text` in the term net `Atom` constructor with a more specialized key type; for example with `ExtCns` names we could use the `VarIndex` for matching. However, this would require modifying the term net library.
brianhuffman
pushed a commit
that referenced
this issue
Apr 28, 2021
Fixes #1263. We now use `identBaseName` instead of `identText` in the `termToPat` definition. While `identText` concatenates the module name and base name, `identBaseName` just returns the base name directly. Using only the base name means that there is a chance of name collisions between constants that differ only by module name. However, term nets are only used as an approximate filter prior to term matching; it is not a problem if a few extra hits are returned from time to time. Ultimately it might be better to replace the use of `Text` in the term net `Atom` constructor with a more specialized key type; for example with `ExtCns` names we could use the `VarIndex` for matching. However, this would require modifying the term net library.
brianhuffman
pushed a commit
that referenced
this issue
Sep 14, 2021
Fixes #1263. We now use `identBaseName` instead of `identText` in the `termToPat` definition. While `identText` concatenates the module name and base name, `identBaseName` just returns the base name directly. Using only the base name means that there is a chance of name collisions between constants that differ only by module name. However, term nets are only used as an approximate filter prior to term matching; it is not a problem if a few extra hits are returned from time to time. Ultimately it might be better to replace the use of `Text` in the term net `Atom` constructor with a more specialized key type; for example with `ExtCns` names we could use the `VarIndex` for matching. However, this would require modifying the term net library.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
During rewriting, the function
toAbsoluteNamefromName.hsis called repeatedly by thetoPatfunction, which is used for matching terms in the rewriter. Profiling of theblstCI tests shows thatidentText(which is called bytoAbsoluteName) performs as much as 6% of all the allocation of the entiresawprogram, because it does a bunch of concatenation of names. This allocation is unnecessary and should be avoided.The text was updated successfully, but these errors were encountered: