Late CC profiling
Simon Peyton Jones
simon.peytonjones at gmail.com
Mon Apr 7 11:37:42 UTC 2025
If you are sure there is no need to tidy the class method selectors then it
would be clearest to have a explicit "materializeImplicitBinds" step in the
pipeline after tidy and before lateCC runs.
Yes, I want to put all the materialisation in one place. I think you are
suggesting Plan A
- Tidy
- Materialise implicit bindings (class op selectors, data constructor
workers, data constructor wrappers)
- Add late cost centres
- Run late plugins
- Prep
- CoreToStg
But then we'll add cost centres to data constructor workers and wrappers.
Or Plan B
- Tidy
- Add late cost centres
- Materialise implicit bindings (class op selectors, data constructor
workers, data constructor wrappers
- Run late plugins
- Prep
- CoreToStg
Now we won't get cost centres on class-op selectors, which Matthew says he
wants.
I suppose we could have do Plan A, but have the "add cost centre" pass skip
over data constructor workers and wrappers.
Do you have any views?
I'm a bit inclined to combine the late-cc/materialise/prep combo into the
main function of GHC.CoreToStg.Prep. But I wonder if the "late plugins"
need to see (a) the materialised implicit bindings and (b) late cost
centres. I have no idea how late plugins are used.
Matthew asks
I would quite like to know if there is an unoptimised call to >>= in my
program. You have probably thought about this a little, so interested to
hear your opinion.
I was thinking that the class-op selector is a small, cheap function, so I
don't mind not profiling it. But perhaps your point is that calling it
symptomatic of running overloaded code, and we might want to know that.
Simon
On Mon, 7 Apr 2025 at 10:37, Andreas Klebinger <klebinger.andreas at gmx.at>
wrote:
> A interesting wrinkle!
>
> I agree with matt that having class methods show up in the profile can be
> very helpful at times.
>
> Constructors and their wrappers on the other hand should be of no concern.
> We explicitly try to avoid wrapping them in cost centres as they generally
> add no useful information to profiles.
>
> I've briefly looked over the code and adding class methods seems to be the
> *first* thing tidy does, not at it's end. (in tidyProgram)
> That would suggest to me that there is a need to tidy these implicit
> bindings so moving them might have further unintended consequences.
>
> But then Note [Injecting implicit bindings] claims we do it at the end of
> tidy, but seems outdated. This seems like a proper mess.
>
> If you are sure there is no need to tidy the class method selectors then
> it would be clearest to have a explicit "materializeImplicitBinds" step in
> the pipeline after tidy and before lateCC runs.
> That seems like it would be clearer and more coherent overall. And it
> would sidestep this issue completely.
>
> Cheers
> Andreas
>
>
> Am 07/04/2025 um 11:08 schrieb Matthew Pickering:
>
> Hi Simon,
>
> It does seem strange (at first glance) that these implicit things happen
> in two different places.
>
> However, I think you should do this refactoring in a separate MR to
> !10479, it seems unrelated to the purpose of that patch and affects other
> things like you have mentioned.
>
> Why do you think the change in callstack is fine? It seems the difference
> is `callstack001` is whether there an entry for `>>=` or not, which seems
> like quite an important thing to place a cost centre on? Andreas has a
> better intuition for this than me. I would quite like to know if there is
> an unoptimised call to >>= in my program. You have probably thought about
> this a little, so interested to hear your opinion.
>
> Cheers,
>
> Matt
>
>
> On Fri, Apr 4, 2025 at 10:44 PM Simon Peyton Jones <
> simon.peytonjones at gmail.com> wrote:
>
>> Hi Matthew, Andreas, Ben
>>
>> In GHC today there is a strange inconsistency:
>>
>> - Bindings for data constructor wrappers and class method selectors
>> are added at the end of CoreTidy.
>> - Bindings for data constructor workers are added at the start of
>> CorePrep
>>
>> This is deeply strange. In !10479 (unary class patch)
>> <https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10479> I had to
>> make some changes in this area and after being baffled I moved all these
>> implicit bindings to one place at the start of CorePrep. That is simpler
>> and neater. There is no reason for them to be separate.
>>
>> BUT I find that one thing happens between Tidy and Prep, namely
>> late-cost-centre injection. That means we will no longer get
>> late-cost-centres for data constructor wrappers and class method
>> selectors. I think that this is what is causing diffs in the output for
>> callstack001 and friends in !10479.
>>
>> I think that this is perfectly fine, and I propose to accept the changes.
>>
>> Another alternative would be to inject all of these bindings at the end
>> of Tidy (instead of the start of Prep), But it's much nicer at the start
>> of Prep -- we don't want any of these bindings to appear in interface
>> files.
>>
>> Can you confirm that the change is ok? Thanks
>>
>> Simon
>>
>
> _______________________________________________
> ghc-devs mailing listghc-devs at haskell.orghttp://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-devs/attachments/20250407/3dc8723d/attachment.html>
More information about the ghc-devs
mailing list