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

Assortment of optimizations #5213

Draft
wants to merge 9 commits into
base: trunk
Choose a base branch
from
Draft

Assortment of optimizations #5213

wants to merge 9 commits into from

Conversation

ChrisPenner
Copy link
Contributor

@ChrisPenner ChrisPenner commented Jul 11, 2024

Overview

Adds a couple optimizations and some debugging stuff.

  • Adds deepTime which times not only the monadic action, but also deeply forces the returned argument. This gives a much more accurate depiction of the amount of time a given thing takes.
  • Removes unsafeTime, I tested it, and as we presumed, it's just broken.
  • Adds enough NFData instances for me to use deepTime on a Names object, since that's where most of my profiling work has been lately 🙃 ; we can add more as necessary.
  • Fixes some Debug weirdness where pTrace'ing strings would just erase part of them.
  • Adds deepEvaluate to the Debug module, it can help with determining when something is slow.
  • Speeds up TDNR lookup from O(freeVars * names) to ~freeVars * log(names)
  • Replaces a linear Names.filter in slurping with a set difference operation

Implementation notes

The only one that has an interesting implementation is findMatchingTermSuffixes; so I'd recommend checking that out.

Test coverage

Benchmarked against 0.5.24, I cloned nimbus, then edit.namespace, here's the time it took to parse the scratchfile:

  • ucm (0.5.23) :
    • First run: 29.1 s (cpu), 22.3 s (system)
    • Second run: 25.1 s (cpu), 19.0 s (system)
  • cp/project-root:
    • First run: 21.9 s (cpu), 14.8 s (system)
    • 12.3 s (cpu), 8.73 s (system)
  • cp/optimizations-2 (optimized):
    • First run: 16.0 s (cpu), 15.1 s (system)
    • Second run: 10.3 s (cpu), 9.90 s (system)

So not a crazy difference from project-root, but still saves us a second or so.

@ChrisPenner ChrisPenner changed the title Cp/optimizations 2 Assortment of optimizations Jul 11, 2024
Copy link
Contributor

mergify bot commented Jul 11, 2024

⚠️ The sha of the head commit of this PR conflicts with #5041. Mergify cannot evaluate rules on this PR. ⚠️

@@ -163,12 +165,12 @@ debugM flag msg a =
debugLog :: DebugFlag -> String -> a -> a
debugLog flag msg =
if shouldDebug flag
then pTrace msg
then trace msg
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Calling pTrace on something that's already a string, tries to parse it like Show output and ends up hiding bits and pieces of it randomly :'(

Base automatically changed from cp/project-root to trunk July 11, 2024 20:45
@aryairani
Copy link
Contributor

@ChrisPenner Any idea why CI / run transcripts (windows-2019) (pull_request) failed on reflog transcript? Shouldn't that include the code that hides it?

@ChrisPenner
Copy link
Contributor Author

Oh strange; I wonder if it inserted them so close that they have the same timestamp and sqlite is sorting them out of order 🤔

BTW not even sure if this PR is worth merging, conceptually this should be a speed up, but the real-world numbers aren't super significant.

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.

2 participants