Skip to content

Improve displayed names in profiles#5861

Merged
aryairani merged 9 commits intotrunkfrom
topic/profiling-names
Sep 9, 2025
Merged

Improve displayed names in profiles#5861
aryairani merged 9 commits intotrunkfrom
topic/profiling-names

Conversation

@dolio
Copy link
Contributor

@dolio dolio commented Sep 5, 2025

This PR modifies some of the compilation process to keep track of names for floated terms. These names are then fed into the profile rendering so that you see more comprehensible call sites, rather than just opaque hash prefixes.

As an example, the code:

bigLoop = do
  f = cases
    0 -> ()
    n ->
     (n -> let
        g = cases
          0 -> ()
          n -> h (n-1)
        g (n-1)) n


  h = cases
    0 -> ()
    n -> f (n-1)

  f 10000000
  printLine "OK"

produces profiling output like this:

           Costs
  Inherited      Local    Function Call Tree
    100.00%      0.00%    #8b4em8qdms
    100.00%      0.00%      bigLoop
     18.54%     18.54%        bigLoop$f
     23.26%     23.26%        bigLoop$f$Lambda0
     30.70%     30.70%        bigLoop$h
     27.51%     27.51%        bigLoop$f$Lambda0$g

The $ separator was chosen to avoid conflicting with multi-part namespaces, which could show up in the profile output.

The remaining hash is the 'watch expression' from the unison file, which has no name.

dolio and others added 3 commits September 5, 2025 19:11
@dolio
Add support for local names in profiling output
8c4d49c
@dolio
Improve profiling names for in-codebase terms
ab37d81 
Use the 'original reference map' to translate created variables back
to their references, so that the reference names can be pretty
printed. This should yield their actual codebase names.
@dolio @github-actions
automatically run ormolu
111aa3d
@aryairani
Copy link
Contributor

aryairani commented Sep 6, 2025

Looks like some type errors at build time

dolio added 3 commits September 8, 2025 18:05
@dolio
Fix a bug introduced when floating was changed
d73c135 
In the lambda floating path, the variables wasn't been added to the
unfresh variable set, so the same variable could be chosen multiple
times erroneously.
@dolio
Fix test code w/r/t lamLift changes
6f22252
@dolio
Add a transcript that runs the profiler
c7582cd 
Doesn't do anything interesting. This is just trying to make sure it
isn't completely broken.
@dolio
Copy link
Contributor Author

dolio commented Sep 8, 2025

Fabio noticed a bug that I fixed in d73c.... Ran base test afterwards and all passed.

@dolio
Cache floated names in EvalCtx
b4cddcc 
Previously you would only get floated names for newly loaded definitions
from the codebase, so profiling anything that was already loaded would
just show hashes. Now we store names for everything we've loaded, like
we do with references and whatnot.

This is also less awkward than the extra result for the floated names from
various functions.
@dolio
Copy link
Contributor Author

dolio commented Sep 9, 2025 •
edited
Loading

I don't really understand the CI failures. It looks like it's complaining about the output for profiling commands in the help transcript. But I didn't change that.

Is this a 'needs to merge with trunk' problem?

Edit: nevermind, I reran the wrong transcripts. Still not sure how it was this PR that failed them.

@aryairani
Copy link
Contributor

aryairani commented Sep 9, 2025

Sorry about the CI failures, it's something that changed nondeterministically in trunk.

@aryairani aryairani merged commit efc6014 into trunk Sep 9, 2025
29 of 31 checks passed
@aryairani aryairani deleted the topic/profiling-names branch September 9, 2025 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aryairani aryairani aryairani approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dolio @aryairani